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

Amplify documentation and add example #65

Merged
merged 3 commits into from
Jul 7, 2023
Merged

Conversation

Slushee-a
Copy link
Contributor

Made this merge request as mentioned in #64.

Why

Coming into the crate I wasn't sure where DisplayInterface and Rgb565 came from, as well as which modules I needed to import. Browsing through different crate docs and looking at clippy helps you figure out all of that, but adding more, and less simplified examples saves you most of that trouble.

Changes included

  • Amplify (and fix) existing example on docs homepage
  • Add example on docs homepage
  • Add list of display-interface crates that can be used to create the DisplayInterface
  • Add usage example to .orientation() and .setpixel()
  • Add usage example to Builder docs page
  • Add an /examples folder (Contains an example for the pi pico with the ili9341)

Could be improved:

  • Missing usage example for display.set_pixels() (It's not very clear what "anything that can provide IntoIterator<Item = u16>" is supposed to be)
  • Explanation on display.set_scroll_region() isn't very extensive/clear
  • Another example on the /examples folder with another display, microcontroller and interface used. Could also include a more advanced drawing rather than just the whole screen being one color. A colored square would be nice.

@Slushee-a Slushee-a requested a review from almindor as a code owner July 7, 2023 18:40
@Slushee-a
Copy link
Contributor Author

I see what went wrong.

  1. Some code on the codeblock goes over the standard formatting limit
  2. I didn't ignore the codeblocks

I will fix this in my next commit very soon

@Slushee-a
Copy link
Contributor Author

Slushee-a commented Jul 7, 2023

I'm really sorry... This is embarassing. Sorry for wasting your time. Apparently it failed because some trailing spaces? otherwise I can't see what made the pipeline failed. Fixed in the latest commit.

@almindor
Copy link
Owner

almindor commented Jul 7, 2023

I'm really sorry... This is embarassing. Sorry for wasting your time. Apparently it failed because some trailing spaces? otherwise I can't see what made the pipeline failed. Fixed in the latest commit.

No problem at all. You're giving something to improve the library, there's nothing to feel sorry or embarrased about.

Copy link
Owner

@almindor almindor left a comment

Choose a reason for hiding this comment

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

Thanks for all the improvements!

@almindor almindor merged commit eff911f into almindor:master Jul 7, 2023
6 checks passed
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