Skip to content

Add wifi.radio.tx_power #6542

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

Merged
merged 4 commits into from
Jul 1, 2022
Merged

Add wifi.radio.tx_power #6542

merged 4 commits into from
Jul 1, 2022

Conversation

todbot
Copy link

@todbot todbot commented Jun 30, 2022

Resolves #6540

@jepler
Copy link

jepler commented Jun 30, 2022

@todbot I revised the initial comment on this PR using the special syntax for closing an issue when a PR is accepted.

@todbot
Copy link
Author

todbot commented Jun 30, 2022

Thanks!

@askpatrickw
Copy link

askpatrickw commented Jun 30, 2022

I actually started to work on this myself, and the get, set in Radio.c made sense. I also thought tx_power would need to go in ports/espressif/common-hal/wifi/Radio.h, is that not the case?

I had no idea shared-bindings would need updating, so I'm glad you took this on, ty!

@todbot
Copy link
Author

todbot commented Jul 1, 2022

I was also confused by no function decls in ports/espressif/common-hal/wifi/Radio.h. I think the rationale is that shared-bindings contains the C API headers and the individual ports' headers contain only platform-specific stuff. So, the Radio.h in shared-bindings is the header for every implementation of wifi.radio.

And apologies for stepping on your work on this! And thanks for the extra set of eyes. I'm always nervous about touching this repo publicly. :)

@askpatrickw
Copy link

You didn't step on me... I don't know C so it would have been a steep learning curve.
I'll test this later tonight or tomorrow during the Deep Dive window.

Thank you again!

Copy link
Collaborator

@dhalbert dhalbert left a comment

Choose a reason for hiding this comment

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

Thanks! One doc clarification requested.

@tannewt
Copy link
Member

tannewt commented Jul 1, 2022

I was also confused by no function decls in ports/espressif/common-hal/wifi/Radio.h. I think the rationale is that shared-bindings contains the C API headers and the individual ports' headers contain only platform-specific stuff. So, the Radio.h in shared-bindings is the header for every implementation of wifi.radio.

This is exactly right. Another way to think about it is that the shared-bindings header declares all of the functions used by the shared-bindings implementation.

Copy link
Collaborator

@dhalbert dhalbert left a comment

Choose a reason for hiding this comment

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

Thanks! Ready to merge when build is done.

@dhalbert dhalbert merged commit 8814ee0 into adafruit:main Jul 1, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add wifi.radio.tx_power for espressif port
5 participants