shared-bus icon indicating copy to clipboard operation
shared-bus copied to clipboard

Replace specific proxy types I2cProxy and AdcProxy by generic type Proxy

Open DrTobe opened this issue 2 years ago • 4 comments

I have seen that the different proxy types (I2cProxy, SpiProxy and AdcProxy) are essentially the same types. The only difference for the SpiProxy is that should only be initialized with the NullMutex. So I guess this can be reduced to only two types there.

Also, the SpiProxy::_u field had no apparent use so I removed it. Have I overlooked something there?

This is WIP because I have not adjusted the documentation and removed the acquire_adc method yet.

DrTobe avatar Nov 18 '21 11:11 DrTobe

Also, the SpiProxy::_u field had no apparent use so I removed it. Have I overlooked something there?

Please read #8 for why this is necessary right now. This is also essentially the reason why we currently have different types for each bus - as you noticed this wouldn't be necessary if they are all the same. I have plans on how to build a better solution for this, but this is still in the works...

Rahix avatar Nov 18 '21 14:11 Rahix

I have understood that the SpiProxy must be its own type which is !Send + !Sync. Therefore, I had only joined I2cProxy and AdcProxy into the generic Proxy type and left the SpiProxy. In general, this just makes the distinction between buses that can be shared between threads/contexts and those which can not.

Still, I do not see the use of the _u: core::marker::PhantomData<*mut ()>.

DrTobe avatar Nov 18 '21 17:11 DrTobe

I have understood that the SpiProxy must be its own type which is !Send + !Sync. Therefore, I had only joined I2cProxy and AdcProxy into the generic Proxy type and left the SpiProxy.

Ah, sorry, I did not see this during my initial read through your changeset. Joining just the "good" proxies is okay. However, we should really keep the old names available as to not require a breaking release. Please add type-aliases for I2cProxy and AdcProxy so code relying on these names still compiles. Similarly, keep the acquire_i2c() and acquire_adc() methods. You can mark all these as deprecated to encourage people to switch.

Still, I do not see the use of the _u: core::marker::PhantomData<*mut ()>.

That's what makes the SpiProxy !Send. Without it, you can "safely" move a proxy to a different thread still (when constructed with a mutex type which allows this).

Rahix avatar Nov 21 '21 11:11 Rahix

keep the old names available as to not require a breaking release

Sounds reasonable, done in the commit before

That's what makes the SpiProxy !Send

I suspected this was the intention. But I guess it is not required because the acquire_spi method is only implemented on the BusManager<NullMutex>:

impl<T> BusManager<crate::NullMutex<T>> {
    pub fn acquire_spi<'a>(&'a self) -> crate::SpiProxy<'a, crate::NullMutex<T>> {
        crate::SpiProxy {
            mutex: &self.mutex,
        }
    }
}

So there will only ever be SpiProxy<'a, crate::NullMutex<T>> instances which are !Send + !Sync because of

// from the docs
impl<'a, M> Send for SpiProxy<'a, M>
where
    M: Sync

impl<'a, M> Sync for SpiProxy<'a, M>
where
    M: Sync, 

and the NullMutex is !Sync.

DrTobe avatar Nov 22 '21 10:11 DrTobe