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

Rework to make use of embedded-hal and no-std in order to get it working on a MCU #1

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

Conversation

mariusknaust
Copy link

Thanks for your great work!

I reworked the library to make use of embedded-hal and no-std in order to get it working on a ARM Cortex-M3.

Should still be working on Linux as before (see test). But I could only test it on the MCU, since I don’t have a Linux device with the sensor attached to it available currently. Please test it on your Linux device before merging.

@mariusknaust
Copy link
Author

Moving the delay doesn’t allow a second sensor depending on the delay as well.

@braincore
Copy link
Owner

Was closing this accidental?

@braincore
Copy link
Owner

Thanks a lot for this! I have a few i2c libraries that I've been meaning to make available for MCUs, but I haven't had time to go through and understand embedded-hal/no-std in detail.

Do you have any resources that you recommend for me to quickly catch up? Whether it's documentation or other repos that do it well?

Overall, your changes look good. You're abstracting out i2c & sleep which are clearly hardware specific. I'll address them soon once I do a little reading.

@mariusknaust
Copy link
Author

mariusknaust commented Mar 2, 2019

No, it was on purpose. I only tested it on it’s own before doing the PR. When I tried to use it with another i²c sensor I realized that moving the delay into it might be a bad idea, since they might need it as well. I’m new to embedded-hal (and rust) too and was looking at other repos (that did the same mistake). I plan to fix that, but I am busy the next week, that’s why I close it. I will reopen it when I have a fix ready.

@braincore
Copy link
Owner

Do you have an example of this being done in a way that wouldn't block progress? Looking at linux_embedded_hal, I only see the sleep approach. No signs of an async / event loop / future / promise system.

@mariusknaust
Copy link
Author

I think the embedded_hal::timer provides a non-blocking way, but I haven’t looked into that yet.

@mariusknaust
Copy link
Author

It seems like sharing a bus between different drivers is a known problem with embedded-hal (issue 35), so it might be a good idea to wait until they figure it out.

@braincore
Copy link
Owner

Wow, that's quite a shortcoming for such a common use case. I agree to wait. Also, check out https://github.com/Rahix/shared-bus if you haven't.

@mariusknaust mariusknaust reopened this Jun 5, 2020
@mariusknaust
Copy link
Author

mariusknaust commented Jun 5, 2020

I’ve successfully used the shared-bus now and changed the functions to borrow the delay instead of moving it into the device, so it should be ready to be used in parallel to other devices (on the same bus).

@braincore
Copy link
Owner

@mariusknaust Sorry for the delay! I'll take a look in the next few days.

By the way, would you like admin/write permissions for this repo?

@mariusknaust
Copy link
Author

Don't worry, take your time.

Sure, why not. I would look forward to to integrate async/await as soon as the infrastructure is ready [1].

[1] https://github.com/rust-embedded-community/async-on-embedded

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