Skip to content

at86rf2xx: correct rssi#9510

Merged
kYc0o merged 1 commit intoRIOT-OS:masterfrom
Josar:pr/at86rf2xx_rssi
Aug 20, 2018
Merged

at86rf2xx: correct rssi#9510
kYc0o merged 1 commit intoRIOT-OS:masterfrom
Josar:pr/at86rf2xx_rssi

Conversation

@Josar
Copy link
Contributor

@Josar Josar commented Jul 6, 2018

This PR changes the the rssi value to be based on the ED and not on the RSSI value as stated in the manuel for the extended operation mode of the 23x devices.
The ifdefs are reordered to to be extended more easier.

Issues/PRs references

#9172

Copy link
Contributor

@kYc0o kYc0o left a comment

Choose a reason for hiding this comment

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

Minor nitpick. Can you link directly to the page in the reference manual where this is stated?

}

return pkt_len;
return pkt_len;
Copy link
Contributor

Choose a reason for hiding this comment

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

Please readd the 4 spaces here.

rssi = 3 * rssi;
#endif
radio_info->rssi = RSSI_BASE_VAL + rssi;
DEBUG("[at86rf2xx] LQI:%d high is good, RSSI:%d high good or interferer.\n",
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't get what "high good or interferer" means?

Copy link
Contributor

Choose a reason for hiding this comment

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

That this value represents either a good link or a lot of interference. Maybe a rephrasing like high is either good or too much interference?

@Josar
Copy link
Contributor Author

Josar commented Jul 18, 2018

@kYc0o directly link?

     * AT86RF231 MAN. p.89 8.3.2 Reading RSSI
     * AT86RF232 MAN. p.88 8.3.2 Reading RSSI
     * AT86RF233 MAN. p.99 8.4.2 Reading RSSI

@Josar Josar force-pushed the pr/at86rf2xx_rssi branch from 55d3eee to 5357e7b Compare July 18, 2018 13:01
@kYc0o
Copy link
Contributor

kYc0o commented Jul 18, 2018

That should be enough! Thanks!. Even better if added to the commit message, so they become part of the git history.

@Josar
Copy link
Contributor Author

Josar commented Jul 18, 2018

I can do this when squash and rebase. Should i do this now?

@kYc0o
Copy link
Contributor

kYc0o commented Jul 18, 2018

I can do this when squash and rebase

Agreed.

@Josar Josar force-pushed the pr/at86rf2xx_rssi branch from 5357e7b to 66f421f Compare July 18, 2018 13:19
@Josar
Copy link
Contributor Author

Josar commented Jul 18, 2018

@kYc0o done.

@Josar
Copy link
Contributor Author

Josar commented Jul 25, 2018

@kYc0o further remarks?

@kYc0o
Copy link
Contributor

kYc0o commented Jul 25, 2018

Not on my side. ACK.

@kYc0o kYc0o self-assigned this Jul 25, 2018
@kYc0o kYc0o added Area: network Area: Networking Area: doc Area: Documentation Type: cleanup The issue proposes a clean-up / The PR cleans-up parts of the codebase / documentation 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 labels Jul 25, 2018
@kYc0o kYc0o added this to the Release 2018.10 milestone Jul 25, 2018
AT86RF231 MAN. p.89 8.3.2 Reading RSSI
AT86RF232 MAN. p.88 8.3.2 Reading RSSI
AT86RF233 MAN. p.99 8.4.2 Reading RSSI

"It is not recommended reading the RSSI value when using the Extended  Operating Modes, use ED instead"
@Josar Josar force-pushed the pr/at86rf2xx_rssi branch from 20ddec6 to 4482c98 Compare July 26, 2018 11:37
@Josar
Copy link
Contributor Author

Josar commented Jul 30, 2018

@kaspar030 could you have a second look if your concerns are addressed?

@Josar
Copy link
Contributor Author

Josar commented Aug 20, 2018

Can this be merged?

@kYc0o
Copy link
Contributor

kYc0o commented Aug 20, 2018

I'd say yes.

@kYc0o kYc0o merged commit 861fbe9 into RIOT-OS:master Aug 20, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Area: doc Area: Documentation Area: drivers Area: Device drivers Area: network Area: Networking CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR Type: cleanup The issue proposes a clean-up / The PR cleans-up parts of the codebase / documentation

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants