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

Refactor control station/keypad handling for RA3/QSX #114

Draft
wants to merge 10 commits into
base: dev
Choose a base branch
from

Conversation

cbw
Copy link
Contributor

@cbw cbw commented Sep 11, 2022

The way keypad button and button LED handling for RA3/HomeWorks QSX extended the Castea data structures in ways that were not straightforward, and had a number of kluges (like stuffing a bunch of data in the name field with underscore delimiters).

This PR refactors the handling of control station devices. It places the control station device (e.g. keypad) into the device tree, and then button groups, buttons, and LEDs are children to that device. This will be unpacked in Home Assistant to create the appropriate devices for keypads, and entities under them for buttons and button LEDs.

These changes do not impact Pico remotes on Caseta. Caseta Pico device handling will require some thought as it will be a breaking change, but would logically fall into this new model.

Tested under current Home Assistant Core dev branch and existing Caseta functionality is unaffected.

Tested successfully against both Home Assistant Core dev branch (doesn't break existing capabilities) as well as my pending HA Core changes to enable this functionality.

@cbw cbw marked this pull request as ready for review September 11, 2022 05:49
@cbw cbw marked this pull request as draft September 11, 2022 05:52
@cbw cbw marked this pull request as ready for review September 11, 2022 20:18
@cbw
Copy link
Contributor Author

cbw commented Sep 11, 2022

@danaues this is working well against my QSX processor and a Caseta Bridge Pro, mind giving it a try against your RA3? Corresponding HA Core is in this branch: https://github.com/cbw/homeassistant-core/tree/keypad_refactor

@danaues
Copy link
Contributor

danaues commented Sep 11, 2022

Yes for sure. I’ll start testing this evening.

@danaues
Copy link
Contributor

danaues commented Sep 12, 2022

@cbw only 1 issue i'm seeing so far. The device triggers for the keypads and picos aren't functioning under the current ha integration for RA3/HWQSX. They appear to be working fine for caseta.
I"ll dig into more this evening to figure out why.

** just realized I'm running the current dev HA. I'll switch to your branch and re-test.

@danaues
Copy link
Contributor

danaues commented Sep 12, 2022

Device triggers confirmed not working for picos and keypads. Also getting some naming duplication in the devices. Each entity is still getting it's own device, instead of being cascaded under 1 keypad device, containing all the leds, buttons.

The keypad naming isn't giving me enough info to know which one it is.
In this case "Main Stairs Position 1" isn't descriptive enough.
I can't tell if this is the Main Floor Main Stairs, or Upstairs Main Stairs or Basement Main Stairs
I think it helps to say "Keypad" or "Pico" in it as well. When selecting devices from the dropdown during automation creating, it really helps to identify them"

image

I'll do some more extensive testing this evening.

@cbw
Copy link
Contributor Author

cbw commented Sep 12, 2022

@danaues the Caseta working makes sense, its Picos are following the existing codepath. Only the "LEAP2" (need a name for the RA3/QSX flavor of LEAP!) devices follow the new code path.

Could you download diagnostics for the RA3 integration and drop that to me on Discord so I can see how it's discovering your system so I can figure out the naming?

What's your event trigger config look like for Picos and keypads on RA3? If you go into Developer Tools > Events > subscribe to lutron_caseta_button_events do you see them firing? I was able to successfully test RA3 keypads and Picos through the Lutron app (unfortunately don't have any physical hardware other than a processor yet), but they all fired ok, I suspect it's just the automations you're using are looking for something different.

@danaues
Copy link
Contributor

danaues commented Sep 13, 2022

@cbw, maybe you should mark this PR as a draft for now, until we've got it all tested

@cbw
Copy link
Contributor Author

cbw commented Sep 13, 2022

@danaues I think things are fine on the pylutron side, but no point in merging until it's working end to end. I'll revert it to draft.

@cbw cbw marked this pull request as draft September 13, 2022 15:42
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