Dedicated GPIO bundle initial implementation #4819
Conversation
…o make it easier to read
…s. But not seem to be working
ttzytt
changed the title
There was a problem hiding this comment.
Pull request overview
This PR implements dedicated GPIO bundles for ESP Rust HAL, building on the dedicated GPIO foundation from PR #4699. It provides a way to control multiple dedicated GPIO channels simultaneously, along with minor API improvements and enhanced documentation.
Changes:
- Implements three bundle types (
DedicatedGpioInputBundle,DedicatedGpioOutputBundle,DedicatedGpioFlexBundle) for efficient multi-channel operations - Updates existing dedicated GPIO APIs to improve usability (removes
mutrequirement fromlevel()methods, enhances debug assertions with clearer error messages) - Adds comprehensive documentation with examples and low-level function helpers
Reviewed changes
Copilot reviewed 2 out of 3 changed files in this pull request and generated 12 comments.
| File | Description |
|---|---|
| hil-test/src/bin/gpio.rs | Adds conditional compilation for architecture-specific features, new test cases for bundle functionality, and infrastructure for multi-core testing |
| esp-hal/src/gpio/dedicated.rs | Implements bundle types, adds low-level helpers (write_ll, read_all_ll, output_levels_ll), improves documentation, and conditionally disables output_level methods on ESP32-S3 due to LLVM bug |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot <[email protected]>
Co-authored-by: Copilot <[email protected]>
Co-authored-by: Copilot <[email protected]>
Co-authored-by: Copilot <[email protected]>
Co-authored-by: Copilot <[email protected]>
Co-authored-by: Copilot <[email protected]>
Co-authored-by: Copilot <[email protected]>
Thank you for the careful review! Of course I don’t mind at all. The notes are really helpful. They already helped me spot a bunch of issues and ambiguities I hadn’t realized, I left some suggestions unresolved and will work on these soon. |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 3 changed files in this pull request and generated 6 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
esp-hal/src/gpio/dedicated.rs
Outdated
| /// Re-enables a dedicated output driver in this bundle after it was disabled by [`Self::disable_output`]. | ||
| /// | ||
| /// This is equivalent to [`Self::with_output`]. | ||
| pub fn enable_output<'d>(&mut self, out: &'lt DedicatedGpioOutput<'d>) -> &mut Self { |
There was a problem hiding this comment.
We can probably get rid of with_output in favor of enable/disable. Otherwise you'd have to explain the difference between them (there is none, really), and explain when to use one over the other.
There was a problem hiding this comment.
Sure, that makes sense.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 3 changed files in this pull request and generated 6 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot <[email protected]>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 3 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
2 participants
This PR builds on the dedicated GPIO foundation introduced in #4699 to implement dedicated GPIO bundles, which allow controlling multiple dedicated channels at once.
This PR also introduces several minor changes to the existing APIs (see this comment) and documentation.
cc #2649