rnilssoncx / homebridge-pico

Expose Lutron Pico Remotes in Homebridge: https://github.com/nfarina/homebridge
MIT License
63 stars 6 forks source link

Crashes on button press with PJ2-4B-GWH-S31 #27

Closed akosut closed 3 years ago

akosut commented 3 years ago

I recently installed a new PJ2-4B-GWH-S31 (my first of this type), and configured it with homebridge-pico by selecting the "PJ2-4B-XXX-S31" remote type. However, this causes Homebridge to crash when I press the buttons. Sample log message below.

I quickly realized that my Pico is generating event types 8, 9, 10 and 11 for the four buttons where the S31 is configured in the plugin source code for 12, 9, 10 and 13. I switched the configuration type from "S31" to "L31" and now everything works fine.

My installation is now stable, but it appears there's a compatibility issue between the remote currently for sale and the configuration in the plugin.

Example log:

[7/20/2021, 3:10:56 PM] [homebridge-pico] [10.0.11.89] Bus Data: ~DEVICE,5,11,3
[7/20/2021, 3:10:56 PM] [homebridge-pico] [10.0.11.89] Device 5 Button 11 - Created tracker
[7/20/2021, 3:10:56 PM] [homebridge-pico] [10.0.11.89] Bus Data: ~DEVICE,5,11,4

/usr/lib/node_modules/homebridge-pico/accessory.js:103
    this.buttons[button].getCharacteristic(Characteristic.ProgrammableSwitchEvent).setValue(click);
                         ^
TypeError: Cannot read property 'getCharacteristic' of undefined
    at PicoRemote.trigger (/usr/lib/node_modules/homebridge-pico/accessory.js:103:26)
    at Pico.clickHandler (/usr/lib/node_modules/homebridge-pico/index.js:80:45)
    at Click._finished (/usr/lib/node_modules/homebridge-pico/index.js:136:10)
    at listOnTimeout (internal/timers.js:557:17)
    at processTimers (internal/timers.js:500:7)
[7/20/2021, 3:10:57 PM] [homebridge-pico] Child bridge process ended
[7/20/2021, 3:10:57 PM] [homebridge-pico] Process Ended. Code: 1, Signal: null
[7/20/2021, 3:11:04 PM] [homebridge-pico] Restarting Process...

homebridge-pico v1.0.2 Homebridge v1.3.4 Node.js v14.17.1 NPM v6.14.13

rnilssoncx commented 3 years ago

It's very possible it was defined incorrectly. I don't believe it would ever change on the Lutron side.

When I have a few minutes I'll fix the definition (and add some needed validation so it doesn't crash on these types of issues).

Thanks for the input on this!

lilyball commented 3 years ago

I just hit this with my first remote, a P2J-4B-XXX-L01, which is just the basic 4-button light switch.

lilyball commented 3 years ago

My buttons are 8,9,10,11. accessory.js actually maps these to the “scene” pico type and says mine should be 2,5,6,4.

lilyball commented 3 years ago

I took a look at accessory.js. If you group all the remotes by the number of buttons they have, within each group, any given button number shows up in exactly one place. By that I mean for all 4-button remotes, if button 5 is present, it’s always the second one. Given that, for any given DEVICE event, the button ID can be mapped to a button position regardless of remote type* as long as the number of buttons are known. Doesn’t matter which 4-button remote you configured, button ID “5” is always the second button.

Given that, the only reason to even distinguish between remotes beyond number of buttons is for providing button labels.

Taken together, I think the config can be replaced with a much simpler approach, where each remote has two questions:

  1. How many buttons are on it?
  2. How do we want to label the remote? This can just be “scene”, “lights”, or “shades”, where with the 5-button remote the third one is always “Favorite”. This is just used to figure out what labels to provide for the buttons.

Not only will this make it easier to configure as I don’t have to know my exact model number, it also means I can map two remotes of different types to the same remote accessory. In fact, that should work even if they have different numbers of buttons, because the two-button remotes are identical to the first/last buttons of all* other remotes, and the three-button remotes are just the two-button ones with the “Favorite” button (which is always the center button).

*There actually is a set of remotes that break this pattern, which are the “2-group controls” remotes, such as PJ2-4B-XXX-L21, PJ2-4B-XXX-S21, and PJ2-4B-XXX-LS21. These could be handled by just adding separate types for them, e.g. “2 lights on/off”, “4 lights on/off”, and “lights+shades”. These 2-group lights would behave differently than previously described when encountering DEVICE button IDs that don’t belong to them, in that they would just ignore (and log) those events as they can’t be reasonably mapped onto the buttons. But for all other remote types, all button events should be mapped uniquely onto one of the buttons.

I also note that there are other remote types not listed as supported (for example, this just declares PJ2-2B and assumes it’s PJ2-2B-XXX-L01). With this scheme as long as the button IDs are known, they can be mapped to physical buttons. And the type will let us know how to label the buttons on the HomeKit side.

rnilssoncx commented 3 years ago

I will have an update shortly that allows for custom remotes, and that won't crash on unexpected button presses.

I don't plan to rearchitect the plug-in at this time, but these observations are interesting.

rnilssoncx commented 3 years ago

Update is out - 1.1.1-beta.0. It no longer crashed on bad button presses. Also allows for custom buttons and button configs.