Skip to content

Conversation

@jessebraham
Copy link
Member

Just some quick cleanup:

  • Only define/implement the types which are relevant for the configured device
    • Currently all 3 radios are in the documentation for all chips, this fixes that
  • Remove the unnecessary RadioExt trait
    • Since RADIO is a virtual peripheral it's already defined in this package, and we don't need an extension trait

@bjoernQ
Copy link
Contributor

bjoernQ commented Oct 17, 2023

I think I like this better and also it probably makes things easier.

But looking at it I was wondering if - given those are just virtual peripherals - we could just define up to three virtual peripherals like Wifi, Bluetooth and Ieee802154 independently. Would probably make things even easier but not sure if that would have any disadvantages (I guess not)

@MabezDev
Copy link
Member

But looking at it I was wondering if - given those are just virtual peripherals - we could just define up to three virtual peripherals like Wifi, Bluetooth and Ieee802154 independently. Would probably make things even easier but not sure if that would have any disadvantages (I guess not)

I like that idea too, I don't think there is any downside to it because there is currently no situation where we don't call split (no esp-wifi api takes the RADIO peripheral directly).

@jessebraham
Copy link
Member Author

Ahh yeah that seems obvious in hindsight haha, thanks for the suggestions. Will update this PR.

@jessebraham jessebraham changed the title Re-work and re-organize the radio module, eliminate the RadioExt trait Replace the radio module with peripheral singleton structs Oct 19, 2023
Copy link
Contributor

@JurajSadel JurajSadel 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
Contributor

@bjoernQ bjoernQ 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

@MabezDev MabezDev 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!

@MabezDev MabezDev merged commit 62a174f into esp-rs:main Oct 20, 2023
@jessebraham jessebraham deleted the fixes/radio branch October 20, 2023 13:22
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.

4 participants