search

sinara-hw / booster-firmware

Other
4 stars 3 forks source link

if take mux #3

Closed hartytp closed 4 years ago

hartytp commented 5 years ago

You use this pattern a lot in the fw if (lock_take(I2C_LOCK, portMAX_DELAY)) but it doesn't really make sense because: (a) as I understand it, lock_take can never fail, just block forever (b) you don't raise any error if lock_take does fail (if it did then bad things could happen in several places).

If you're sure that lock_take can never fail then the if statement should be removed to make the code more readable.

Out of curiosity (forgive me, it's been ages since I wrote C) can't you use a macro to wrap your I2C functions in a lock_take ... lock_free pair automatically? This (a) reduces boiler plate (b) means you can't accidentally forget to release the lock

hartytp commented 5 years ago

You could also put the mux select in the macro so that you don't need to worry about implementing it by hand each time (reduces number of places you can make a mistake)