search

tinygo-org / drivers

TinyGo drivers for sensors, displays, wireless adaptors, and other devices that use I2C, SPI, GPIO, ADC, and UART interfaces.
https://tinygo.org
BSD 3-Clause "New" or "Revised" License
608 stars 192 forks source link

Add support for ams AS560x on-axis magnetic rotary position sensors (#510) #520

Closed neildavis closed 1 year ago

neildavis commented 1 year ago

Resolves #510. I think it's time to get some eyes on this :)

Issues I'd appreciate opinions on:

  1. How many 'convenience' methods (wrappers around lower-level register read/write methods) should we include in the API? I've included only what I think are likely to be commonly used and/or where they can 'add value' (e.g. angle unit conversions, decoding of STATUS for magnet position/strength, BURN count limit logic) All device functionality is available via the lower-level register access methods, but should we wrap them all?
  2. An open TODO: in the Angle(units AngleUnit) method when the value is requested in degrees and MPOS/MANG have been set. See the comment in the method. Maybe just drop conversion to degrees from the driver and let the caller do it?
neildavis commented 1 year ago

Marking as 'draft' for now as tackling the second issue from my opening comment may lead to a major refactor. Feedback is still welcome though if anyone has the time and interest!

neildavis commented 1 year ago

I think this is ready for review now. Thanks to @sago35 for pointing me at the TinyGo dev branch which resolved my USB serial debugging issues on my Tiny2040, I was able to further investigate the AS5600's behavior around the interaction of setting ZPOS/MPOS/MANG. Whilst this logic is not particularly clear from the datasheet, and introduces a little more complexity than we'd like, I think it's best to handle this in the driver where I can document it rather than every client program struggling to deal with with it!

deadprogram commented 1 year ago

I made a few comments @neildavis but they are just coding style things. You obviously know a lot about these devices, thank you very much for working on this!

neildavis commented 1 year ago

I made a few comments @neildavis but they are just coding style things. You obviously know a lot about these devices, thank you very much for working on this!

@deadprogram Many thanks for the review, and in particular the hint on avoiding allocs when returning errors. Great spot!

I've made all the suggested changes and rebased against latest dev branch.

deadprogram commented 1 year ago

The research on this device is pretty impressive @neildavis thanks for all the work! Now merging.