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 i2c Component #198

Merged
merged 3 commits into from
Dec 21, 2023

Conversation

jrh4567
Copy link
Contributor

@jrh4567 jrh4567 commented Dec 20, 2023

This pull request adds the Adafruit I2C Stemma QT Rotary Encoder Breakout with NeoPixel to Wippersnapper_Components.

Product URL: adafruit.com/product/4991

I set the subcomponents to "unspecified" and "raw" for the encoder value and button, respectively - I didn't see anything better in the list of i2c sensor types.

Copy link
Contributor

@tyeth tyeth left a comment

Choose a reason for hiding this comment

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

Hi @jrh4567 👋 Thanks for your contribution, it all looks fine for now!
We haven't actually got a great option for output type that would suit this device. I had thought we were going to add a boolean option to Adafruit_Sensor, but maybe that wasn't needed yet. I'm also unsure what effect using unspecified will have. If it was me I would use raw and raw, then also specify a display name for each subcomponent, in effect having two blocks similar to https://github.com/adafruit/Wippersnapper_Components/blob/main/components/i2c/stemma_soil/definition.json#L11-L14

Also good work on making it past the CI checks. I see you've also cloned the main WipperSnapper repository, hopefully you've found the guide about contributing to WipperSnapper.
Are you planning to write the software driver / code for this component too? We (@brentru and I) will gladly help you with any questions you have, maybe start a draft (draft if you think it's not ready yet) pull request over in the main repository with your first code / thoughts / attempt and we can discuss anything related to the codebase in more detail there.

Copy link
Contributor

@tyeth tyeth left a comment

Choose a reason for hiding this comment

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

Apologies for the change in review status, but I think it would make more sense to have raw and raw, with appropriate labels for button and the encoder

@jrh4567
Copy link
Contributor Author

jrh4567 commented Dec 21, 2023

Hi @tyeth! Thanks for your message, I've updated definition.json as you requested. It was definitely going to be confusing trying to figure out the difference between unspecified and raw.

Yes, I am going to take a stab at writing the driver, following the instructions on the guide - it looks like I'll just be porting the arduino library to C? I'll create a draft PR if I have any questions. Thanks!

@tyeth tyeth merged commit 154d77f into adafruit:main Dec 21, 2023
13 checks passed
@tyeth
Copy link
Contributor

tyeth commented Dec 21, 2023

@jrh4567

Okay, that's now merged and available in the component picker, but it will have the In-Development banner to warn users that it's functionality/driver is not included in the latest release (or may be unstable/untested).

Previously our component picker had a checkbox to show the In-Development components, but with the new searchable redesign currently all components are initially shown. This will not be the case in the new year so you may need to select dev components as part of the component filtering in the future. You can also search for 'dev' to show all dev components in the picker (you'll notice it also includes the Analog 'Dev'ices ADT7410 component):
image

@jrh4567
Copy link
Contributor Author

jrh4567 commented Dec 21, 2023

Awesome, thanks @tyeth!

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.

2 participants