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

spi/adc_st7789_example #125

Merged
merged 18 commits into from
Nov 8, 2022
Merged

spi/adc_st7789_example #125

merged 18 commits into from
Nov 8, 2022

Conversation

monacoprinsen
Copy link
Contributor

Added an SPI example, drawing an image on the ST7789 screen.

@monacoprinsen monacoprinsen changed the title spi_st7789_example spi/adc_st7789_example Sep 10, 2022
@monacoprinsen
Copy link
Contributor Author

Added ADC examples

@ivmarkov
Copy link
Collaborator

@monacoprinsen I think it would be great to have an ADC example! Ideally though, examples should be tiny and targeted to demoing just one feature. So I think it would actually be beneficial if you remove the st7789 aspects from this example. (Or create another example, that is only demonstrating how to write on an st7789 screen.)

The other thing is, would it be possible to update the example so that it follows the latest changes to the master branch? There were quite a few of those, but the ADC driver is mostly intact, modulo some renames here and there?

@monacoprinsen
Copy link
Contributor Author

@ivmarkov thanks for the input.
I have added 3 examples:
Simple adc example
St7789 adc example
St7789 spi example

If you prefer that I remove the St7789 examples I can do that.

Should work to update to the latest branch, will do that.

@ivmarkov
Copy link
Collaborator

@monacoprinsen The "St7789 adc" example we should remove, the other two we keep.
Can you please also update the examples to be compatible with latest master? They are currently failing the cargo fmt but I looked briefly at them, and they are not based on master.

@monacoprinsen
Copy link
Contributor Author

@ivmarkov
Alright will do that!

@monacoprinsen
Copy link
Contributor Author

@ivmarkov now it should be up to date.

@ivmarkov
Copy link
Collaborator

@ivmarkov now it should be up to date.

Can you make sure that the CI checks pass successfully on your changes? I.e. cargo fmt, cargo clippy and the subsequent actual build. Otherwise, merging your changes will immediately break CI and I need to immediately follow up with fixes of your just merged code.

@monacoprinsen
Copy link
Contributor Author

Will do

@monacoprinsen
Copy link
Contributor Author

@ivmarkov sorry for the delay.... It should be ok now.

@ivmarkov
Copy link
Collaborator

@ivmarkov sorry for the delay.... It should be ok now.

Compile fails.

@monacoprinsen
Copy link
Contributor Author

@ivmarkov ok will look into that.

@ivmarkov
Copy link
Collaborator

The path to the ferris image seems incorrect?

@monacoprinsen
Copy link
Contributor Author

@ivmarkov thanks, adjusted it!
Sorry for the mess, hopefully next contribution will be a little cleaner.

@ivmarkov
Copy link
Collaborator

ivmarkov commented Nov 8, 2022

Thank you!

@ivmarkov ivmarkov merged commit 8cd8c8c into esp-rs:master Nov 8, 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.

2 participants