-
Notifications
You must be signed in to change notification settings - Fork 434
Add ssd1309 to README #303
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
Conversation
rust-highfive
commented
Nov 5, 2020
Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @adamgreig (or someone else) soon.
If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. Due to the way GitHub handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes.
Please see the contribution instructions for more information.
Thank you for developing this driver @antonok-edm!
I really do not want to downplay your effort, however, I see it was added to the released driver list and I ask myself whether this driver is really release-ready.
All I can see is just one commit in the repository. There is also no blog (or similar) announcement post like we traditionally do.
@eldruin No worries! To be honest, hidden behind that one commit is a lot of hard work by @jamwaffles, who's already made some very solid APIs for interacting with displays like these. I started from the code in rust-embedded-community/sh1106@294e8fa and my changes primarily consist of changing command specifications and the init sequence to support the ssd1309
, which has slightly different requirements.
The driver already works very well on my own SSD1309 SPI display, I figured having the driver posted somewhere may be useful to anyone else with the same display.
Regarding a blog/announcement post, I just noticed there are some other drivers with GitHub links, so I just copied that format. I don't have much of a blog myself, but if that's a requirement I could set something up on my website.
Hmm, honestly I think the link to github is unnecessary since it is trivial to get there from crates.io.
I have noticed now that there are several cases of this in the list but the criteria is right there:
The list below contains drivers developed as part of the Weekly Driver initiative and that have achieved the "released" status (published on crates.io + documentation / short blog post).
I guess they slipped through.
Anyway I do not want to make a big fuss about it because this is not really important but I think a short higher level announcement / demonstration post in whatever form is valuable for the community since not everybody can jump right into some code.
Hey @antonok-edm, If you have not found the time for the higher-level post, you could always add this driver to the WIP section just beneath it.
It would be great to be able to promote this work by having it somewhere in the list.
10d77d3
to
b0dc37e
Compare
b0dc37e
to
35c8e39
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great! thank you for following through with this.
There was a small edit mishap but I will fix it myself.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
bors r+
Build succeeded:
👍🏼
I've forked the
sh1106
crate to make a new driver crate for SSD1309 monochrome OLED displays - I figured it would be good to add to this list as well.cc @jamwaffles - this may be of interest to you as well 😄