search

eldruin / mlx9061x-rs

Platform-agnostic Rust driver for MLX90614/MLX90615 Infrarred thermometer
Apache License 2.0
6 stars 5 forks source link

Support embedded-hal 1.0 #7

Closed ShakenCodes closed 4 months ago

ShakenCodes commented 5 months ago

Move to the Embedded-HAL 1.0 driver, which has some changes to the I2C interface. Also, ensure compatibility with the embedded-hal-bus constructs.

I intend to add this support in the coming week. Just adding the issue as a notification. Feel free to reach out if there is anything to discuss.

eldruin commented 5 months ago

Sounds good to me, thanks!

ghislaine-laios commented 5 months ago

Is there any update?

ShakenCodes commented 5 months ago

I've got a couple of tasks in the queue ahead of this... Probably will start early next week.

ghislaine-laios commented 5 months ago

I've got a couple of tasks in the queue ahead of this... Probably will start early next week.

Do you need help? I'm willing to help but only have an MLX90614 on hand.

ShakenCodes commented 5 months ago

Thank you for the offer of help! I created the fork and implemented the changes yesterday evening at: https://github.com/Radiator-Labs/mlx9061x-rs I have hit several testing errors in the actions run, which need to be resolved. My hope is to fix them and submit the PR today. Your suggestions are welcome, of course! The first run is at: https://github.com/Radiator-Labs/mlx9061x-rs/actions/runs/9104910268

ShakenCodes commented 5 months ago

I am stuck. The tests are failing because the I2CMock now requires done() to be called as part of the destruction and emits a warning when they are not destroyed. The test cases that test creation with an illegal slave address fail, as there is no driver returned for the test-case to destroy. Can you suggest how to fix this? https://github.com/Radiator-Labs/mlx9061x-rs/actions/runs/9104910268

One possible approach is to make SlaveAddr unable to be created with an illegal value. This fits the design philosophy of never allowing a type to be in an illegal state. This would be a breaking change, because SlaveAddr would need to be created with either SlaveAddr::default() or SlaveAddr::fallible_new(u8). I'm not overly worried about making this a breaking change, as there is already breaking changes in creating the I2C object being handed in.

ShakenCodes commented 4 months ago

Got feedback from the embedded_hal_mock author. Am testing a fix, now...

ShakenCodes commented 4 months ago

PR is submitted! I have not bumped the version in the Cargo.toml (I think of this as the maintainer's decision... although I'm not really sure what proper protocol is!

ghislaine-laios commented 4 months ago

Got feedback from the embedded_hal_mock author. Am testing a fix, now...

I've been busy today and only just had time to check your work now. I'm happy to hear that the problem has been resolved.