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

Add ESP-NOW Support #7470

Merged
merged 18 commits into from
Mar 10, 2023
Merged

Add ESP-NOW Support #7470

merged 18 commits into from
Mar 10, 2023

Conversation

microdev1
Copy link
Collaborator

ESP-NOW is a proprietary wireless communication protocol which supports direct communication between espressif devices.
This would be of particular interest for low-power wireless communication requirements.

This PR is based on the previous work by @nickzoic, @shawwwn, @glenn20 and contributions from @zoland, including:

Closes #3999.
Closes glenn20#4.

Co-authored-by: Nick Moore <[email protected]>
Co-authored-by: shawwwn <[email protected]>
Co-authored-by: glenn20 <[email protected]>
@microdev1 microdev1 added enhancement network espressif applies to multiple Espressif chips labels Jan 20, 2023
@microdev1 microdev1 marked this pull request as draft January 20, 2023 08:39
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.

@microdev1 Thank you for taking this on! We certainly have gotten a lot of call for it.

I realize this is a draft, and you know there are things left to do, so many of my comments are more just to make a TODO list that can be checked off.

Targeting this for 9.0.0 might make sense, since then we can use ESP-IDF v5.0 from the start. I will set it to that milestone for now.

py/ringbuf.h Outdated Show resolved Hide resolved
py/ringbuf.c Outdated Show resolved Hide resolved
ports/espressif/bindings/espnow/ESPNow.c Outdated Show resolved Hide resolved
ports/espressif/bindings/espnow/ESPNow.c Outdated Show resolved Hide resolved
ports/espressif/bindings/espnow/ESPNow.c Outdated Show resolved Hide resolved
ports/espressif/bindings/espnow/ESPNow.c Outdated Show resolved Hide resolved
ports/espressif/bindings/espnow/ESPNow.c Outdated Show resolved Hide resolved
ports/espressif/bindings/espnow/ESPNow.c Outdated Show resolved Hide resolved
ports/espressif/bindings/espnow/ESPNow.c Outdated Show resolved Hide resolved
ports/espressif/bindings/espnow/ESPNow.c Outdated Show resolved Hide resolved
@dhalbert dhalbert added this to the 9.0.0 milestone Jan 20, 2023
@glenn20
Copy link

glenn20 commented Jan 23, 2023

@microdev1 Thanks also for taking this on. I confess I couldn't work out where to get started porting this to circuitpython. If there is anything I can do to help, let me know.

@microdev1 microdev1 marked this pull request as ready for review January 23, 2023 18:22
@microdev1 microdev1 modified the milestones: 9.0.0, 8.0.0 Jan 23, 2023
@microdev1
Copy link
Collaborator Author

...If there is anything I can do to help, let me know.

@glenn20 (and others) Sure, will let you know and thanks for all the work you have done already. 🙂

Targeting this for 9.0.0 might make sense...

I am hoping this can be in CP8. The ESP-NOW API doesn't seem to have changed in [email protected].

@dhalbert
Copy link
Collaborator

I am hoping this can be in CP8. The ESP-NOW API doesn't seem to have changed in [email protected].

We could do for 8.x.x. We are trying to close out 8.0.0 really soon, but we have a bunch of things for 8.x.x.

@dhalbert dhalbert modified the milestones: 8.0.0, 8.x.x Jan 23, 2023
Copy link
Member

@tannewt tannewt left a comment

Choose a reason for hiding this comment

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

Thank you for porting this over!

Do you want to keep it MP compatible?

ports/espressif/bindings/espnow/ESPNow.c Outdated Show resolved Hide resolved
ports/espressif/bindings/espnow/ESPNow.c Outdated Show resolved Hide resolved
ports/espressif/bindings/espnow/ESPNow.c Outdated Show resolved Hide resolved
ports/espressif/bindings/espnow/ESPNow.c Outdated Show resolved Hide resolved
ports/espressif/bindings/espnow/ESPNow.c Outdated Show resolved Hide resolved
ports/espressif/bindings/espnow/ESPNow.c Outdated Show resolved Hide resolved
ports/espressif/bindings/espnow/__init__.c Outdated Show resolved Hide resolved
@glenn20
Copy link

glenn20 commented Jan 28, 2023

Do you want to keep it MP compatible?

While I'm not the port-er, I'd be interested in maintaining compatibility with the original module on MP. I'm happy to adopt agreed changes to the api into the MP module if required.

@tannewt
Copy link
Member

tannewt commented Jan 31, 2023

@glenn20 Sounds good. I'd love your feedback on my inline comments then.

@microdev1
Copy link
Collaborator Author

I have reworked the API. There are still some changes to be made in the underneath implementation and a few checks to be run but the API will largely remain the same and I'd like to get your opinion on it.

@nickzoic
Copy link

nickzoic commented Feb 2, 2023

Hey this is cool, thanks @microdev1 and @glenn20 for picking it up and running with it!
(also g'day to @tannewt and @dhalbert !)

Copy link
Member

@tannewt tannewt left a comment

Choose a reason for hiding this comment

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

Thank you for the updates! @glenn20 and @nickzoic please let me know what you think of my API suggestions.

ports/espressif/bindings/espnow/ESPNow.c Outdated Show resolved Hide resolved
ports/espressif/bindings/espnow/ESPNow.c Outdated Show resolved Hide resolved
ports/espressif/bindings/espnow/ESPNowPacket.c Outdated Show resolved Hide resolved
ports/espressif/bindings/espnow/ESPNowPacket.c Outdated Show resolved Hide resolved
ports/espressif/bindings/espnow/__init__.c Show resolved Hide resolved
ports/espressif/bindings/espnow/__init__.c Outdated Show resolved Hide resolved
@microdev1
Copy link
Collaborator Author

In the ESP-NOW packet the message comes after a wifi_promiscuous_pkt_t and a espnow_frame_format_t.
Currently only the rssi is parsed from wifi_promiscuous_pkt_t, thoughts on exposing it entirely as a wifi.Monitor packet?

@microdev1 microdev1 modified the milestones: 8.x.x, 8.1.0 Feb 18, 2023
ports/espressif/bindings/espnow/ESPNow.c Show resolved Hide resolved
ports/espressif/bindings/espnow/ESPNow.c Show resolved Hide resolved
ports/espressif/bindings/espnow/Communicate.c Outdated Show resolved Hide resolved
ports/espressif/bindings/espnow/Communicate.c Outdated Show resolved Hide resolved
@microdev1 microdev1 requested a review from tannewt March 9, 2023 06:35
Copy link
Member

@tannewt tannewt left a comment

Choose a reason for hiding this comment

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

One minor doc fix, otherwise the API looks good. Thank you!

ports/espressif/bindings/espnow/ESPNow.c Outdated Show resolved Hide resolved
Co-authored-by: Scott Shawcroft <[email protected]>
@microdev1 microdev1 requested a review from tannewt March 9, 2023 18:32
Copy link
Member

@tannewt tannewt left a comment

Choose a reason for hiding this comment

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

Looks good to me!

@tannewt tannewt requested a review from dhalbert March 10, 2023 17:13
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, looks good to me too!

@dhalbert dhalbert merged commit b6a7613 into adafruit:main Mar 10, 2023
@microdev1 microdev1 deleted the espnow branch March 10, 2023 17:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement espressif applies to multiple Espressif chips network
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Developing ESP-NOW for circuitpython Support ESP32-NOW
7 participants