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 support for Grafik Eye QS keypad #44

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

kevineriklee
Copy link

I tested this with the Grafik Eye QS attached to my RadioRA2 main repeater and it appears to work.

A few design decisions that are worth discussing:

  • I prepended "ColumnN" to the name of any button that appears in a shade column. This is because when events are fired in Home Assistant, they can only be distinguished based on the button name, so I need these to be unique. I imagine that most other people will have identical button names across the multiple shade columns (at the very least, Raise and Lower will be the same since they have no engraving), so I'd argue we need some way to distinguish them.
  • I kept "Unknown Button" the same without prepending "ColumnN". This is because Home Assistant specially checks for that string, so I didn't want to break that functionality for people.
  • There is no rhyme or reason to the mapping of button and LED component IDs for the Grafik Eye QS, and there is no way to determine it by examining the XML (that I could tell), so I had to resort to hard coding a bunch of stuff. I think this was done in a nice way, but let me know if you have suggestions.

Copy link
Owner

@thecynic thecynic left a comment

Choose a reason for hiding this comment

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

small nit, but otherwise looks good.

Copy link
Owner

@thecynic thecynic left a comment

Choose a reason for hiding this comment

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

Feel free to reupload, I haven't heard back since last time i commented.

@kevineriklee
Copy link
Author

Sorry life got busy and I didn't get a chance to look at this for a while...

Addressed your comments

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