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

Raspi example #133

Open
wants to merge 15 commits into
base: master
Choose a base branch
from

Conversation

PatrickLang
Copy link

@PatrickLang PatrickLang commented Aug 4, 2020

PR description

This merges in a community sample for using the library on a Raspberry Pi, along with a test cross build using GitHub Actions which are free for public open source repos. This was based on a sample shared in #48 which worked well for me.

What do you think about the organization? I can add a few more missing details on how to wire up the lcd and do other cleanup if needed.

Work remaining

  • CircleCI build step for this sample
  • Ensure this sample uses the current version of ssd1306 from this repo, not an older released version

Other checks

  • Check that you've added documentation to any new methods
  • Rebase from master if you're not already up to date
  • Add a CHANGELOG.md entry in the Unreleased section under the appropriate heading (Added, Fixed, Changed, etc)
  • Run rustfmt on the project with cargo fmt --all - CI will not pass without this step
  • Check that your branch is up to date with master and that CI is passing once the PR is opened

Not applicable

  • Add or modify an example if there are changes to the public API

@jamwaffles
Copy link
Collaborator

Thanks for the PR! I think the example might be better placed under examples/raspi as opposed to the current top-level examples-raspi. I'd also prefer not to use two different CI pipelines, so if you're familiar with CircleCI please move the build there. If not, I can take a look at adding the cross based build into CircleCI for you.

@PatrickLang
Copy link
Author

Sure will update the layout and take a look at circleci. I have used it before

@PatrickLang
Copy link
Author

I changed the paths, but still need to add CircleCI. I should be able to do that by end of week.

@PatrickLang
Copy link
Author

@chux0519 - does this look ok to you? I just added more details in the readme. I think my OLED pinout matches yours but I wanted to double check.

I will do the circleCI update soon


let i2c = I2cdev::new("/dev/i2c-1").unwrap();

let mut disp: GraphicsMode<_> = Builder::new().connect_i2c(i2c).into();
Copy link

@slyshykO slyshykO Aug 6, 2020

Choose a reason for hiding this comment

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

In new 0.4.0 version Builder have no connect_i2c method.

Copy link
Author

Choose a reason for hiding this comment

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

ah yes that's a good point. examples/raspi/cargo.toml uses 0.3.something. I should update that too.

Copy link
Author

Choose a reason for hiding this comment

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

This should be fixed. I updated a bunch of dependencies and worked around the breaking changes

@chux0519
Copy link

chux0519 commented Aug 6, 2020

@PatrickLang It's been a while since I wrote that, and all code looks good to me.
I think as long as the pipeline passed(not breaking current version), it should be ok

@PatrickLang
Copy link
Author

Sorry it took me a bit to get back to this. I've been fighting with Jenkins in my day job and haven't been in the mood to work on CI at home ;) I got the sample up to the current library revision and it builds in CircleCI.

Just a note - you may want to move the other build steps to cimg/rust which will supercede circleci/rust

Copy link
Collaborator

@jamwaffles jamwaffles left a comment

Choose a reason for hiding this comment

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

Hi Patrick, my turn to apologise! I'm sorry I'm an entire year late on getting back to this... lockdowns and mental health struggles have kept me away from doing much OSS work in this time.

Anyway, many thanks for the example and writing up an awesome little tutorial doc with images! I've left a few small comments inline, but the version changes I brought up will need some larger changes to the example code itself.

If you're happy to make the changes, please feel free, otherwise can you switch on the "allow contributors to push to branch" option or whatever its called so I can get this PR merged in? Failing that, I can fork this PR with updates :).

Also please rebase this branch if you have a moment.

I'll do my best to respond in a more timely fashion this cycle!

Comment on lines +25 to +27
[target.armv7-unknown-linux-gnueabihf]
linker = "arm-linux-gnueabihf-gcc"

Copy link
Collaborator

Choose a reason for hiding this comment

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

Is this really required? I don't have this package installed on my M1 mac and the rpi example compiled with no errors.

@@ -93,6 +93,10 @@ fn HardFault(ef: &ExceptionFrame) -> ! {

```

### Raspberry Pi

A sample project is available under [examples/raspi](./examples/raspi)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Nit

Suggested change
A sample project is available under [examples/raspi](./examples/raspi)
A sample project is available under [examples/raspi](./examples/raspi).

embedded-graphics = "0.6.0"
linux-embedded-hal = "0.3.0"
machine-ip = "0.2.1"
ssd1306 = "0.4.0"
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think this should be a path directive to keep the example code here the same as everywhere else:

Suggested change
ssd1306 = "0.4.0"
# NOTE: The `path` directive is used here to keep the example up to date. Use a normal version
# number in real application code.
ssd1306 = { path = "../../" }

# See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html

[dependencies]
embedded-graphics = "0.6.0"
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
embedded-graphics = "0.6.0"
embedded-graphics = "0.7.1"

although this breaks some stuff in the example code.

@jamwaffles jamwaffles mentioned this pull request Nov 12, 2021
6 tasks
Copy link

@btielen btielen left a comment

Choose a reason for hiding this comment

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

I would like to suggest to add a file examples/raspi/.cargo/config with

[build]
target = "armv7-unknown-linux-gnueabihf"

to avoid the problem mentioned in #48 (comment)

@patricklangsonos
Copy link

Thanks! I need to pull my Raspberry Pi back out and make some updates. I put this on hold for a while to work on other projects but may need it again soon.

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.

None yet

6 participants