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

[link-metrics] add missing initialization of noisefloor for link metrics #695

Merged
merged 1 commit into from
Nov 28, 2023

Conversation

Irving-cl
Copy link
Contributor

The PR adds missing initialization of noise floor in link metrics
module. Without the PR, the link margin obtained in link metrics
enhanced-ack probing is incorrect.

On simulation platform, the initialization is done:
https://github.com/openthread/openthread/blob/main/examples/platforms/simulation/radio.c#L503

@Irving-cl Irving-cl added the bug Something isn't working label Nov 27, 2023
Copy link
Contributor

@zhanglongxia zhanglongxia left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Member

@edmont edmont left a comment

Choose a reason for hiding this comment

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

LGTM, thanks.

However, wouldn't be less error prone if the Link Metrics module would initialize the noise floor itself? I.e., using SubMac::GetNoiseFloor.

@Irving-cl
Copy link
Contributor Author

LGTM, thanks.

However, wouldn't be less error prone if the Link Metrics module would initialize the noise floor itself? I.e., using SubMac::GetNoiseFloor.

SG. That needs adding a new platform API and SPINEL method to set the noise floor at the RCP. I can do it later.

@jwhui jwhui changed the title [link metrics] add missing initialization of noisefloor for link metrics [link-metrics] add missing initialization of noisefloor for link metrics Nov 28, 2023
@jwhui jwhui merged commit a265943 into openthread:main Nov 28, 2023
7 checks passed
@edmont
Copy link
Member

edmont commented Jan 17, 2024

SG. That needs adding a new platform API and SPINEL method to set the noise floor at the RCP. I can do it later.

Hi @Irving-cl, any update on this?

@Irving-cl
Copy link
Contributor Author

@edmont Sorry I'm quite busy these days. Would you like to add it if you have time?

@edmont
Copy link
Member

edmont commented Jan 18, 2024

@Irving-cl I'm also busy, just checking if changes were expected around this, but not hard requirement. Let's leave it for sometime later.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants