-
Notifications
You must be signed in to change notification settings - Fork 42
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
More complete keypad support needed #10
Comments
I may have addressed some of these in an enhanced-button-support branch on my fork from some time back, which I never submitted since #8 didn't get traction, and because I never got to two last items on my refactoring checklist, which I'll address with your third question.
Supported.
Supported. (More in response to question 3 below).
Would need to spend some time refreshing myself on the branch to determine if this is supported. I suspect it's not, but not far off.
Not supported. It looks like LED state could be set through the TELNET interface, but I'd proceed with caution. Providing my thoughts on your questions:
The LED's state is managed separately from the button (e.g., an "All Off" button's LED will illuminate when the scene is active, regardless of if the button is pressed), so my opinion is that it's helpful to have these in a separate object, which allows you to more cleanly query the controller. That being said, you'll see I added it as a member on the Button.
My opinion is that an application should need to subscribe to the button or LED individually. As an example, it became apparent that the "LED" more accurately represents the state of a "scene", and the button is just the trigger to turn the "scene" on. If you want to subscribe to a "scene" going active or inactive, regardless of what triggered the state change, you'll want to be able to subscribe to the LED.
There is some "Special Handling" for buttons in the branch I previously linked. Cleaning this up was the last item on my refactor list. With this handling, you can subscribe to a button, and the handler will be invoked whenever a button is pressed. You can then interrogate the object to find out what button was pressed -- you don't need to pass "state," per se, because the event is a "button press" not a "state change." I'm using this to trigger automation in home-assistant when buttons are pressed (unfortunately that contribution isn't merged either, given the dependency on this library). The final refactoring I wanted to close before submission was to enable several subscribers to the same object (event). As it stands, you can only have one handler for each button, light, etc. As you can see, the last push was end of September, so I lost some steam. Let me know if any of this is useful and I can try to at least tie off a few things to submit a PR. |
Thanks for the detailed reply. With regard to subscribing to button events, it looks like the code that you have for handing button events does not distinguish between press, release, hold and multi-tap events on the buttons and nor can a subscriber to events on those buttons. This is what I meant when I said that there is a need to see the details of the event since currently you're neither propagating the action number nor allowing the user to filter on the action number. My inclination would be to offer a different sort of subscription callback for subscriptions to buttons (and their corresponding LEDs) and pass the action and parameters to that callback. |
Thanks for pointing this out. I have a RadioRA 2 system, which doesn't support hold or multi-tap events, and I don't have any shades on the system, and had never seen an "Open" / Action = 4 event, but it wouldn't call the subscriber in the current implementation. I would be a proponent of a generally enhanced subscription system that allowed you to subscribe not just to objects but to specific object events. Rather than propagating the event back to the caller for filtering, enable the caller to subscribe to Button Press, or Button Hold events, and have the library call the correct subscribers. This would not be exclusive to buttons -- it'd be nice to subscribe to output devices only when they reached current states as well rather than any change. |
Yes, you're right that the RadioRA2 doesn't support multi-tap, although it can send hold events. You can also get press/release event pairs from the raise and lower buttons for both lights and shades. I have shades on my system so I can do testing on that. I'd be happy to have the event filtering done by the library rather than having to let the subscriber filter, although I think that you still want to pass the action type to the subscriber since it will often make sense to use one callback function for multiple actions and you will certainly want to pass the action parameters when they exist. This is all the more relevant when you start to add subscriptions for events on output objects too. |
Sorry guys, it;s been quite crazy for me and I haven't been able to get to any of this. Also, these seem to get buried in my gmail box. I'll try to catch up with some of the comments soon. |
I think subscribing to button/led events directly is more in line with what I was thinking originally. I never completed any of that because I don't have any fancy scenes that I trigger from HASS. Could you describe why you need multiple subscribers though? There's a single owner of this library when its instantiated within HASS, or anywhere else. Why can't you vector out in the client callback? Otherwise you have to manage subscriptions here, and then do we support unregistering some but not others? It seems like an unnecessary complication and again I don't see the use case. Could you describe what you want to do? I'd also generally agree that we should just pass the event type to the callback. |
@thecynic Thanks for getting back to us despite things being crazy. I don't think we need multiple subscribers; all I was saying is that we might want multiple types of subscriber and so we needed a richer model of passing context to the subscriber. As well as passing back more information about the event it would be useful to be able to register a context object that would be passed back when the subscriber is notified. |
@nickovs Ok, had some more time to hack on it. Now has a subscription model for outputs, keypads, buttons, leds, etc. You can toggle leds and buttons. Take a look at the dev-buttons-leds branch. I still am not taking a context object because I think you can do that when you create the callback and thus it just adds more clutter. You can either pass a helper from the wrapper class in HASS, or you can add a lambda and bind your context object with a default value arg. Will that work for you? I'm pretty sure it's backwards compatible, but I mostly have been manually testing and haven't hooked anything back up to HASS. Note that RRA2 will override your LED states after ~10 sec unless your configure it in the SW to leave the LEDs alone. Mine all revert back, but I didn't do anything special to them and I do remember seeing something about LED state there. There's a bit of clean up left to do (doc strings, etc.) |
Thanks! I will take a look. Note that on initial inspection I think that you have a problem with your
Accepting a context would be really useful, mostly dues to the way that Python binds variables in closures by reference rather than value. Take for example the simplistic case of:
This yields a list of ten functions all of which return
The problem with this code is that all of the events will get the name of the last area added, not the area for the button concerned. This problem isn't an issue when you subscribe a method of an object that holds all the context but there's not always a relevant HASS
Yes, that's the case for buttons that 'do something'. The much more useful case is for buttons that have not been programmed with a scene and on these the default is for the LED state not to be changes by the RRA2 master repeater. I've been using spare buttons to do things like start and stop the music on Sonos and if the button is not programmed with a scene then the LED can provide feedback on the playing state. Thanks again for your work on this. When I get the chance I'll try your latest code on my HASS installation and let you know if I find any issues. |
I see, after finishing my first coffee of the day, that you did suggest:
so my explanation was probably unnecessary, at least for you. Still, I find that idiom way less straightforward than just having a context object and I also think it's much easier for someone coming to the code later who is not aware of the pass-by-reference pitfall to miss it and break things. The final decision is of course up to you but since you're changing the API to move subscription from the top-level |
On Mar 19, 2018 6:39 AM, "Nicko van Someren" <notifications@github.com> wrote:
I see, after finishing my first coffee of the day, that you did suggest:
you can add a lambda and bind your context object with a default value arg
so my explanation was probably unnecessary, at least for you. Still, I find
that idiom way less straightforward than just having a context object and I
also think it's much easier for someone coming to the code later who is not
aware of the pass-by-reference pitfall to miss it and break things.
The final decision is of course up to you but since you're changing the API
to move subscription from the top-level Lutron object to the LutronEntity
objects now would be the time to add a context if it's ever going to happen.
Yeah, ok fair enough. I was being a bit too NIMBY here. Adding a context
pointer isn't that onerous. I'll see if I can respin tonight.
…--Dima
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#10 (comment)>,
or mute
the thread
<https://github.com/notifications/unsubscribe-auth/ABcEMW7wZzodOZFm_berj8iXMlUEF5eHks5tf7UkgaJpZM4Ra654>
.
|
On Mar 19, 2018 6:20 AM, "Nicko van Someren" <notifications@github.com> wrote:
Ok, had some more time to hack on it. Now has a subscription model for
outputs, keypads, buttons, leds, etc. You can toggle leds and buttons. Take
a look at the dev-buttons-leds branch.
Thanks! I will take a look. Note that on initial inspection I think that
you have a problem with your Button.Event.RELEASED definition
<https://github.com/thecynic/pylutron/blob/dev-buttons-leds/pylutron/__init__.py#L631>
:-)
Copy/paste FTW :) I've never actually seen it generate release events.
Maybe for the shade/raise/lower buttons? Will poke tonight.
…--Dima
I still am not taking a context object because I think you can do that when
you create the callback and thus it just adds more clutter. You can either
pass a helper from the wrapper class in HASS, or you can add a lambda and
bind your context object with a default value arg. Will that work for you?
Accepting a context would be really useful, mostly dues to the way that
Python binds variables in closures by name rather than value. Take for
example the simplistic case of:
adders = [lambda x:x+i for i in range(10)]
This yields a list of ten functions all of which return x+9, because i is 9
when it goes out of scope. In the context of your code, if I want to fire
HASS events when I get button events then I would try:
for area in hass.data[LUTRON_CONTROLLER].areas:
for keypad in area.keypads:
for button in keypad.buttons:
def callback(lutron_button, event, params):
hass.bus.fire(LUTRON_BUTTON_EVENT, {"area":area.name, ...})
button.subscribe(callback)
The problem with this code is that all of the events will get the name of
the last area added, not the area for the button concerned. This problem
isn't an issue when you subscribe a method of an object that holds all the
context but there's not always a relevant HASS Entity for every button.
There are work-arounds, such as adding another class or passing bits of
context in the default values of function parameters, but none of them are
as clean as simply providing context when you register the callback and
getting it back when you are called. It seems to me that we'd be better off
having one set of "clutter" in LutronEntity.subscribe() and
LutronEntity._dispatch_event() than having to have it everywhere that
subscriptions are used. I'm not just trying to pass the buck, honest!
Note that RRA2 will override your LED states after ~10 sec unless your
configure it in the SW to leave the LEDs alone.
Yes, that's the case for buttons that 'do something'. The much more useful
case is for buttons that have not been programmed with a scene and on these
the default is for the LED state not to be changes by the RRA2 master
repeater. I've been using spare buttons to do things like start and stop
the music on Sonos and if the button is not programmed with a scene then
the LED can provide feedback on the playing state.
Thanks again for your work on this. When I get the chance I'll try your
latest code on my HASS installation and let you know if I find any issues.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#10 (comment)>,
or mute
the thread
<https://github.com/notifications/unsubscribe-auth/ABcEMcSmc01UesjjDAS0kcZFFaqaeXmzks5tf7CMgaJpZM4Ra654>
.
|
So "tonight" was optimistic. I finally got some time to poke around. Take a look at the latest commit on the branch. Gives you the context ptr you wanted, and handles released ;-) I'll probably end up collapsing a few commits together (like the WIP and the tip) in a bit, but take a look and see if anything is missing. I'd like to get the motion sensor stuff sorted as that would be nice to export to HASS. Will look at that next. Was going to do that first before pushing a new version to PyPi but if you want the buttons/leds in a released version, I can do that. |
Great. I'll give it a try. I'm happy to try out motion sensor code when you have some if you want. I started some work on this a little while ago but there's not much to it. The only thing to note is that there are two types of Radio RA2 Powr Savr(™) sensor, the OCR2B and the VCR2B. The later only sends events when it has not detected motion for the preset time (i.e. it is only indicating 'vacancy') while the former notifies for both arrival and departure. Also, neither model allows the system to poll for the current state of the sensor (since they turn the radio off when they are not actively signalling). As such I suspect that it's going to be hard to reliably provide a state variable in HASS. It should however be easy enough to provide events that can be used to trigger actions. |
I'm aware :) I have quite a few OCR2Bs sprinkled around the house. I didn't bother with VCR2Bs because OCR2Bs is strictly a superset from my perspective and I didn't want to be restricted. The querying is supported, just probably has a lot of edge cases. For instance, OCR2B reliably produces occupied/unoccupied state, presumably because the master keeps track. I'm going to see what the HASS changes look like for the lights, should be pretty minimal in theory. Have you already got something for buttons/leds? |
Are you successfully querying the occupancy state of individual sensors, or of areas and groups? The current Lutron Integration Protocol guide does not list querying the Powr Savr sensors as an option (page 140, the 'Occupied' and 'Unoccupied' actions are only listed to support '~' for 'Report'). Supporting being able to poll the occupancy state of areas or groups would be helpful but it would also be handy to have occupancy events for each sensor. |
Thank you all for the great work on this! Will anything need to be updated in the HASS component to support these changes? I'd love to help test, but I only have a repeater at this point so won't be that useful. |
Has there been any further development with this? Where do things stand? |
The short answer is no. Some of the foundational work necessary was put in in version 0.2.0 but that was several months ago. I've been meaning to pick this up but I switched jobs so I've been rather busy. It is still something that I would like to have. Perhaps the maintainer will reply to this. |
congrats on the new job nicko :) maybe @bdunks will chime in also? I was working with him outside of GH on a personal branch of his and he had quite a bit working as of a month or 2 ago. |
@nickovs could you look over the current keypad support (lots of things landed here) and see what else you'd like added? I generally don't use most of this stuff so you folks have to tell me what's needed :-) |
Sure. I'll give it a try. |
Not sure if I'm asking this in the right place. I just got Home Assitant running and connected it to my Lutron QS system. I can see and do basic items. The issue I'm posting about is keypad presses triggering events in hassio. The sample and documentation here: https://www.home-assistant.io/integrations/lutron. says I should be able to see keypad presses, but I can not seem to get it to work. Is this working and what is the trick? |
OK, played with it further and it appears in my QS system only events from Pico Keypads are being recognized. seeTouch keypad events are broadcast by QS exactly as the Pico's are but the system is ignoring them. |
All Pico variants works fine for me on RA2. The 6-key with raise and lower, such as W6BRL, do generate events in HA, but all keys are of this format: <>_unknown_buttom. in-between the angle brackets is a valid Lutron keyboard name given by the Lutron integration. Would be pleased to help ... |
The current support for keypads is fairly minimal. @bdunks has put forward a patch #8 to allow the emulation of button presses and I've been working on a separate patch to support subscriber notification of button presses and LED changes. Before I go too far on my patches I thought it would be useful to have a discussion about the best way to represent these components as objects.
As I see it there are several things that would be useful for keypad objects to be able to do:
Open questions that on which I would appreciate input are:
DEVICE
events such as button presses the event is transient rather than stateful and we need to pass the parameters up to the subscriber.Right now I'm inclined to represent buttons and LEDs together, have applications subscribe to buttons individually and define a new subscription model for buttons that passes both button press and LED change events to the subscriber. The subscription model for the existing
LutronEntity
model would remain the same.Any thoughts on these subjects would be very helpful. Otherwise I'll just continue the way I'm going.
The text was updated successfully, but these errors were encountered: