Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

sx127x: 2 fixes and 1 enhancement (replaces #10917) #10932

Merged
merged 4 commits into from Mar 8, 2019
Merged

sx127x: 2 fixes and 1 enhancement (replaces #10917) #10932

merged 4 commits into from Mar 8, 2019

Conversation

korotkoves
Copy link
Contributor

Contribution description

1 bugfix, 1 fix and 1 enhancement for sx127x driver (sx1272/sx1276 Lora chipset)

Bugfix for sending in implicit headers mode
Fix for netdev send call (remove duplicate code + bugfix for sx127x_set_tx())
Enhancement: set chipset's ValidHeader interrupt to NETDEV_EVENT_RX_STARTED netdev event

Testing procedure

To test enhancement, add NETDEV_EVENT_RX_STARTED catching code in event callback function

Issues/PRs references

Replaces PR #10917

@aabadie aabadie added Type: enhancement The issue suggests enhanceable parts / The PR enhances parts of the codebase / documentation Area: drivers Area: Device drivers Area: LoRa Area: LoRa radio support labels Feb 3, 2019
@aabadie aabadie self-requested a review February 3, 2019 19:30
@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 Mar 8, 2019
@aabadie
Copy link
Contributor

aabadie commented Mar 8, 2019

I just tried this PR and I can confirm that RX started is correctly triggered, pkg_semtech-loramac also still works.
ValidHeader detection irq is not enabled in the original Semtech driver (see sx1272 and sx1276) but it's just that it doesn't care about RX started events.
I just amended a few minor style issues and pushed 36affe1 to avoid having the message "Unexpected netdev event received" in tests/driver_sx127x application each time a message is received.

For me, this PR is good to go but maybe @dylad, @jia200x or @leandrolanzieri are also interested in having a look ?

@jia200x
Copy link
Member

jia200x commented Mar 8, 2019

the changes make sense to me. I will test it with #11022

@jia200x
Copy link
Member

jia200x commented Mar 8, 2019

the changes make sense to me. I will test it with #11022

It works fine with #11022.

Copy link
Contributor

@aabadie aabadie left a comment

Choose a reason for hiding this comment

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

Thanks for testing @jia200x, let's merge this one.

ACK and go

@aabadie aabadie merged commit 65fd51c into RIOT-OS:master Mar 8, 2019
@danpetry danpetry added this to the Release 2019.04 milestone Mar 11, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: drivers Area: Device drivers Area: LoRa Area: LoRa radio support CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR Type: enhancement The issue suggests enhanceable parts / The PR enhances parts of the codebase / documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants