Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add STEMMA QT Rotary Encoder Driver #524

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

jrh4567
Copy link

@jrh4567 jrh4567 commented Dec 21, 2023

This pull request adds a (not yet ready) driver file WipperSnapper_I2C_Driver_RotaryEncoder.h for the STEMMA QT Rotary Encoder. I have several questions:

(all in WipperSnapper_I2C_Driver_RotaryEncoder.h)

  1. Line ~60: Does anything special need to happen to tell Adafruit_Seesaw that it's talking to a rotary encoder? Or does it figure itself out?
  2. Line 61: Do I need to send any values for the "flow" and "reset" parameters in the seesaw begin method?
  3. Line 74: I didn't see any sensor events in Adafruit_Sensor.h that seemed like it fit - does the unified sensor library need to be edited to support seesaw devices, or is there another way to do it that I'm not seeing?

@tyeth
Copy link
Contributor

tyeth commented Dec 21, 2023

I think I have lead you astray with the talk of two raw sensor type subcomponents (which I didn't realise wont currently work), and as you've possibly half deduced - each component must match up to one of the sensor types from adafruit_sensor.

We've had a quick discussion and there isn't currently a solution (back to raw+unspecified or other thoughts) that we're happy with releasing, and the best solution to offer you right now and allow your work to continue this side of the holidays is to remove the button raw component and just work on the encoder value for now. In the new year we'll come up with a robust plan / way forward (we want to support clicking the encoder button in addition to the neopixel over i2c).

As far as the seesaw goes, it should be preprogrammed to have the functionality it needs, and the getEncoderPosition should function as expected although calling reset true could be a good idea if the device is being removed (deinitialised) by a user and then added again (init called a second time). Code-wise if it was a simple sensor returning one raw value then you would be effectively just replicating the example reading code (loop) in the getEventRaw method and the setup code in the begin method.However in this case, looking at the Adafruit SeeSaw Examples -> encoders -> encoder_basic (and I was also checking for the quad example)
image
It's going to be more complex than the simple sensor example described in the learn guide, as I assume you need to poll the sensor/encoder continuously and only report the value changes periodically (similar to the UART sensors which are polled differently to the i2c sensors). I'm also curious what range of data values were you thinking of returning for the encoder value?
Fear not, most of these things are just minor obstacles (or good learning opportunities 🥇 ) and we'll help get it over the line.

@jrh4567
Copy link
Author

jrh4567 commented Dec 21, 2023

Should I change it so that it reports as some other kind of sensor (like temperature) for now, and then we can revisit proper seesaw component support in WipperSnapper after the holidays? The genesis for this contribution was a last-minute holiday gift that requires two Feather Huzzah's talking over Adafruit IO - I can do it without WipperSnapper and just use the regular IO library if I need to.

I was going to try to only push the value of the encoder to IO once the value stopped changing (i.e. if the user wanted to move the encoder up 10, the driver would send the value once it had stopped changing instead of after the first click). As for range of sensor values, I hadn't really gotten that far yet.

@tyeth
Copy link
Contributor

tyeth commented Dec 22, 2023

I think that's what my polling talk was about (maybe it was misleading), but like you said only reporting once the changing of the encoder stops. I'll be honest, i've not used the component before (seesaw version) so the accessor method may be all you need (it probably deals with sensible range of values and the rollover), and report that value in getEventRaw (which the main i2c polling method calls into), but it would only be possible currently to report that every 30seconds minimum as the code for i2c sensors stands at the moment. If you still wish to play with it and change it to a single raw subcomponent then I can merge and update that in the morning, having temperature rather than raw wont make much difference.

I think for last minute gifts then you're correct in using the IO library (or the HTTP API or mqtt) directly in either circuitpython or arduino (your flavour of choice) and we can get this component further over the next few weeks (I have been keen on this one for a little while - all the midi talk in the live streams). Good luck with the gift/project. Also worth mentioning the responses will be slower after today (Friday), if at all, until the new year.

@jrh4567
Copy link
Author

jrh4567 commented Dec 22, 2023

Yeah, I'm going to pivot to using the IO library. Thanks for your help, and happy holidays!

@tyeth
Copy link
Contributor

tyeth commented Dec 22, 2023

I've merged the change to the component definition (removing the second raw subcomponent for button), so if you want to play with the encoder component after the gift giving then you're now able to do so.

Happy holidays to you too!

/*******************************************************************************/
bool begin() {
_RotaryEncoder = new Adafruit_seesaw();
return _RotaryEncoder->begin((uint8_t)_sensorAddress); //TODO: Do i need to send constructor params for unit8_t "flow" or bool "reset"?
Copy link
Contributor

@tyeth tyeth Dec 22, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add reset, not sure about flow I'll have to do some reading.
Also we have to pass the i2c bus (TwoWire object passed into the constructor, and here available as _i2c) into the sensor/seesaw in addition to i2c address. Have a play with the arduino encoder-example from adafruit seesaw library and try passing in a Wire object so you get the feel for it. The reason is the QTPY boards use the second TwoWire bus (a.k.a. Wire1) so we have to use the overload of seesaw constructor/begin that supports defining the bus. See here https://github.com/adafruit/Adafruit_Wippersnapper_Arduino/blob/main/src/components/i2c/drivers/WipperSnapper_I2C_Driver_STEMMA_Soil_Sensor.h#L44

otherwise.
*/
/*******************************************************************************/
bool getEncoderPosition(sensors_event_t *rawEvent) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

At the moment this should be getEventRaw like the stemma_soil sensor does, and we'll have similar methods for other event types in the future (or maybe refactor to do encoder / buttons / neopixels differently over i2c). See https://github.com/adafruit/Adafruit_Wippersnapper_Arduino/blob/main/src/components/i2c/drivers/WipperSnapper_I2C_Driver_STEMMA_Soil_Sensor.h#L85

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants