Skip to content

drivers/sx127x: improve netdev adaption#8232

Merged
aabadie merged 3 commits intoRIOT-OS:masterfrom
aabadie:pr/drivers/sx127x_netdev
Dec 10, 2017
Merged

drivers/sx127x: improve netdev adaption#8232
aabadie merged 3 commits intoRIOT-OS:masterfrom
aabadie:pr/drivers/sx127x_netdev

Conversation

@aabadie
Copy link
Copy Markdown
Contributor

@aabadie aabadie commented Dec 9, 2017

While working on #8160 and #7356, I noticed that the current netif adaption requires the netdev interface to provide a NETOPT_DEVICE_TYPE.

This PR adapts the sx127x netdev to make it compatible with this design. It also remove the NETOPT_DEVICE_MODE that I think now is useless.

@aabadie aabadie added Area: drivers Area: Device drivers Area: LoRa Area: LoRa radio support Area: network Area: Networking labels Dec 9, 2017
@aabadie aabadie requested a review from miri64 December 9, 2017 16:37
@aabadie aabadie mentioned this pull request Dec 9, 2017
Merged
@aabadie aabadie added the CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR label Dec 9, 2017
miri64
miri64 requested changes Dec 9, 2017
View reviewed changes
drivers/sx127x/sx127x_netdev.c
if (*(const uint16_t*) val == NETDEV_TYPE_LORA) {
sx127x_set_modem(dev, SX127X_MODEM_LORA);
return sizeof(uint16_t);
}
Copy link
Copy Markdown
Member

@miri64 miri64 Dec 9, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more. 

else {
    return -EINVAL;
}

The return-value -ENOTSUP means the operation is not supported. Here we have an unsupported value.

Copy link
Copy Markdown
Contributor Author

@aabadie aabadie Dec 9, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, I was unsure of that. Will change.

Copy link
Copy Markdown
Contributor Author

@aabadie aabadie Dec 9, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

@miri64
Copy link
Copy Markdown
Member

miri64 commented Dec 9, 2017

This PR adapts the sx127x netdev to make it compatible with this design. It also remove the NETOPT_DEVICE_MODE that I think now is useless.

Maybe not useless, but in this case redundant. Since the option was introduced for LoRA however, I agree, that it can be removed (for now).

@aabadie aabadie changed the title drivers/sx127x: improve device type management in netdev adaption drivers/sx127x: improve netdev adaption Dec 9, 2017
@aabadie
Copy link
Copy Markdown
Contributor Author

aabadie commented Dec 9, 2017

I also pushed a commit that fixes return values with some boolean getters. It was required for #7482

miri64
miri64 approved these changes Dec 9, 2017
View reviewed changes
Copy link
Copy Markdown
Member

@miri64 miri64 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ACK. Please squash.

@aabadie aabadie force-pushed the pr/drivers/sx127x_netdev branch from 38d186c to 6144c1b Compare December 9, 2017 18:58
@aabadie aabadie force-pushed the pr/drivers/sx127x_netdev branch from 6144c1b to c52e11f Compare December 9, 2017 19:07
@aabadie
Copy link
Copy Markdown
Contributor Author

aabadie commented Dec 10, 2017

squashed and now it's all green, so go!

@aabadie aabadie merged commit 04c3acc into RIOT-OS:master Dec 10, 2017
@miri64
Copy link
Copy Markdown
Member

miri64 commented Dec 10, 2017

Welp, you could have used this PR, to fix the uint32_t problem with the channel in #7356 ;-). Guess this should go into another PR.

@miri64
Copy link
Copy Markdown
Member

miri64 commented Dec 10, 2017

I meant #7482.

@aabadie
Copy link
Copy Markdown
Contributor Author

aabadie commented Dec 10, 2017

Guess this should go into another PR.

Indeed or in #7482 directly ?

@aabadie aabadie added this to the Release 2018.01 milestone Jan 18, 2018
@aabadie aabadie deleted the pr/drivers/sx127x_netdev branch February 26, 2018 10:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@miri64 miri64 miri64 approved these changes

Assignees

No one assigned

Labels

Area: drivers Area: Device drivers Area: LoRa Area: LoRa radio support Area: network Area: Networking CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR

Projects

None yet

Milestone

Release 2018.01

Development

Successfully merging this pull request may close these issues.

2 participants

@aabadie @miri64