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 dynamic output gpio example to demonstrate how to use AnyPin and Flex #1903

Draft
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

Szybet
Copy link
Contributor

@Szybet Szybet commented Aug 5, 2024

Submission Checklist 📝

  • I have updated existing examples or added new ones (if applicable).
  • I have used cargo xtask fmt-packages command to ensure that all changed code is formatted correctly.
  • My changes were added to the CHANGELOG.md in the proper section.
  • My changes are in accordance to the esp-rs API guidelines

Extra:

Pull Request Details 📖

Description

I added an example which shows how to use the AnyPin and Flex, because it took me too much time to figure out myself and I don't want for anyone else to waste time on this.

Testing

Measured the GPIOs states on the ESP32-C6-DevKitM-1 using an oscilloscope.

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.

I think this would be better as an improvement to the documentation: https://docs.esp-rs.org/esp-hal/esp-hal/0.19.0/esp32c6/esp_hal/gpio/struct.Flex.html, instead of an example. As an example it's basically the same as the blinky_erased example.

Would you mind changing this PR to improve the docs for Flex instead?

MabezDev

This comment was marked as duplicate.

MabezDev

This comment was marked as duplicate.

MabezDev

This comment was marked as duplicate.

@Szybet
Copy link
Contributor Author

Szybet commented Aug 7, 2024

I think this would be better as an improvement to the documentation: https://docs.esp-rs.org/esp-hal/esp-hal/0.19.0/esp32c6/esp_hal/gpio/struct.Flex.html, instead of an example. As an example it's basically the same as the blinky_erased example.

Would you mind changing this PR to improve the docs for Flex instead?

The point of this was to make it easier to find this solution, adding it to the docs somewhere is not really a solution to what I'm trying to solve

@MabezDev
Copy link
Member

MabezDev commented Aug 7, 2024

The point of this was to make it easier to find this solution, adding it to the docs somewhere is not really a solution to what I'm trying to solve

Maybe I'm missing something, but if the usage is in the docs (remember, we can add code snippets to the docs) it's probably more discoverable than it being in an example?

@Szybet
Copy link
Contributor Author

Szybet commented Aug 8, 2024

Maybe I'm missing something, but if the usage is in the docs (remember, we can add code snippets to the docs) it's probably more discoverable than it being in an example?

To find it in docs you need to know what you are looking for, while examples are more easily discoverable by just the description of what are they doing

@bjoernQ
Copy link
Contributor

bjoernQ commented Sep 6, 2024

I agree with @MabezDev - maybe extending blinky_erased_pins would be an option but another example just for GPIO is somewhat "expensive"

Every example is built for every target in CI (which is run for every commit in every PR). Additionally, there is maintenance effort needed whenever something gets refactored

@Szybet
Copy link
Contributor Author

Szybet commented Sep 6, 2024

Ok, I'm gonna look into it

@Szybet
Copy link
Contributor Author

Szybet commented Sep 9, 2024

Do I just expand it by putting it into Flex and AnyPin and making a comment "You can use it as output now too"?

Also, what's with the name erased pins? I don't get it

@bjoernQ
Copy link
Contributor

bjoernQ commented Sep 9, 2024

"erased pin" = type erased pin - things changed a lot since adding this example and the name probably isn't a great fit anymore

@bugadani
Copy link
Contributor

bugadani commented Sep 9, 2024

Please note that the GPIO driver is under rework, so things will change around here.

@Szybet
Copy link
Contributor Author

Szybet commented Sep 9, 2024

Sure, I will look into it once it's ready

Have you thought maybe about a subset of examples which wouldn't be compiled on every commit (to my knowledge they are compiled with regular cargo build, without release), just cargo check? This example could fit in it, the one from #2032 too.

Or just run the check just before merging a pull request? I understand why we should limit the amount of examples, but I think they are important

@bjoernQ bjoernQ marked this pull request as draft September 12, 2024 06:22
@bjoernQ
Copy link
Contributor

bjoernQ commented Sep 12, 2024

Converted to draft for now - set it to Read-for-Review once it's done

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