Skip to content

nrf802154: fix set_tx_power function#14673

Merged
PeterKietzmann merged 1 commit intoRIOT-OS:masterfrom
jia200x:pr/fix_nrf_pow
Aug 2, 2020
Merged

nrf802154: fix set_tx_power function#14673
PeterKietzmann merged 1 commit intoRIOT-OS:masterfrom
jia200x:pr/fix_nrf_pow

Conversation

@jia200x
Copy link
Copy Markdown
Member

@jia200x jia200x commented Jul 31, 2020

Contribution description

This PR fixes the set_tx_power function of the nrf802154 radio.

There was a missing else in the second if. Thus, the second if would always execute if the first one did.
Thus, when using high values for tx power it writes garbage data to the register.

Testing procedure

Try setting the TX power to high values (above 8 dBm). Increasing the TX power shouldn't degrade the RSSI on a reception node.

Issues/PRs references

Discovered while testing #14655

@jia200x jia200x requested a review from bergzand as a code owner July 31, 2020 15:20
@jia200x jia200x requested a review from a user July 31, 2020 15:20
@PeterKietzmann PeterKietzmann self-requested a review July 31, 2020 15:21
@PeterKietzmann PeterKietzmann added Area: drivers Area: Device drivers Type: bug The issue reports a bug / The PR fixes a bug (including spelling errors) CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR Reviewed: 1-fundamentals The fundamentals of the PR were reviewed according to the maintainer guidelines Reviewed: 2-code-design The code design of the PR was reviewed according to the maintainer guidelines Reviewed: 3-testing The PR was tested according to the maintainer guidelines Reviewed: 4-code-style The adherence to coding conventions by the PR were reviewed according to the maintainer guidelines Reviewed: 5-documentation The documentation details of the PR were reviewed according to the maintainer guidelines labels Jul 31, 2020
PeterKietzmann
PeterKietzmann approved these changes Aug 2, 2020
View reviewed changes
Copy link
Copy Markdown
Member

@PeterKietzmann PeterKietzmann left a comment •
edited
Loading

Choose a reason for hiding this comment

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

ACK. Tested with examples/default on two nrf52840dk boards in the FIT IoT-LAB testbed (nrf52840dk-1 & nrf52840dk-2).

  • Send frame from 1 to 2 using txtsnd.
  • Look at RSSI on received frame on 2.
  • Change txpower on 1 to 10dBm.
  • Check ifocnfig on 1 and look at received frame on 2.

current master

  • 2 receives frames with an RSSI of approx. -58 (0dBm on 1).
  • After changing txpower to 10dBm on 1, ifconfig shows 10dBm which is not a valid setting.
  • Frames are not received on 2 anymore.

this PR

  • 2 receives frames with an RSSI of approx. -58 (0dBm on 1).
  • After changing txpower to 10dBm on 1, ifconfig shows 8dBm.
  • Frames are received on 2 with an RSSI of approx. -48 (8dBm on 1).

@PeterKietzmann PeterKietzmann merged commit c1f83ae into RIOT-OS:master Aug 2, 2020
@PeterKietzmann PeterKietzmann mentioned this pull request Aug 3, 2020
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@PeterKietzmann PeterKietzmann PeterKietzmann approved these changes

@bergzand bergzand Awaiting requested review from bergzand bergzand is a code owner

Assignees

No one assigned

Labels

Area: drivers Area: Device drivers CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR Reviewed: 1-fundamentals The fundamentals of the PR were reviewed according to the maintainer guidelines Reviewed: 2-code-design The code design of the PR was reviewed according to the maintainer guidelines Reviewed: 3-testing The PR was tested according to the maintainer guidelines Reviewed: 4-code-style The adherence to coding conventions by the PR were reviewed according to the maintainer guidelines Reviewed: 5-documentation The documentation details of the PR were reviewed according to the maintainer guidelines Type: bug The issue reports a bug / The PR fixes a bug (including spelling errors)

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@jia200x @PeterKietzmann