rust-embedded / embedded-hal

A Hardware Abstraction Layer (HAL) for embedded systems
Apache License 2.0
2.01k stars 202 forks source link

Implement async trait for i2c::RefCellDevice #600

Closed sjoerdsimons closed 6 months ago

Dirbaio commented 6 months ago

this will panic if you actually try to do two transfers from two async tasks in parallel. First task will borrow the RefCell, second task will panic because it's already borrowed. I'm not sure if we should do this because it'd make it too easy to get panics. Ideally you'd use a "real" async mutex, for example from embassy-sync/embassy-embedded-hal or rtic-sync.

sjoerdsimons commented 6 months ago

Right i didn't consider that as my mental model for anything RefCell is, you're responsible for keeping the reference invariants otherwise panic (e.g. the same would happen if you borrow the bus from the refcell before any i2c transfer) ;).

I'm using this to simply poll three ina230's one by one in an async task^0. Which ofcourse works fine. However indeed this will break quickly if you try to use it in a select! like construct. Which again may simply be something that comes with the RefCell territory.

That said revisiting things I mostly ended up doing the above due to 1:1 porting from sync to async code; somehow missing the embassy-embedded-hal shared_bus module :) Which indeed is a nicer solution, so i guess we can close this PR and hopefully these notes will be useful for some in the future