core icon indicating copy to clipboard operation
core copied to clipboard

Fix lutron caseta Sunnata Keypad support to match actual DeviceType returned by RA3 Processor

Open danaues opened this issue 1 year ago • 10 comments

Proposed change

Adjust the SunnataKeypad naming to match actual DeviceTypes returned by the RA3 processor.

The Sunnata Keypads only return "SunnataKeypad" as the device type. I was overriding this in the pylutron_caseta module, to follow the PICO naming convention. The feedback I received was to not do this, which I agree with in retrospect. I've modified the device maps to just show "SunnataKeypad". All 3 types of keypads will show all the buttons under trigger options, but it still works just fine. I'm open to suggestion if you can think of a better/cleaner way to handle this.

Type of change

  • [ ] Dependency upgrade
  • [X] Bugfix (non-breaking change which fixes an issue)
  • [ ] New integration (thank you!)
  • [ ] New feature (which adds functionality to an existing integration)
  • [ ] Deprecation (breaking change to happen in the future)
  • [ ] Breaking change (fix/feature causing existing functionality to break)
  • [ ] Code quality improvements to existing code or addition of tests

Additional information

Checklist

  • [X] The code change is tested and works locally.
  • [X] Local tests pass. Your PR cannot be merged unless tests pass
  • [X] There is no commented out code in this PR.
  • [X] I have followed the development checklist
  • [X] The code has been formatted using Black (black --fast homeassistant tests)
  • [X] Tests have been added to verify that the new code works.

If user exposed functionality or configuration variables are added/changed:

If the code communicates with devices, web services, or third-party tools:

  • [ ] The manifest file has all fields filled out correctly.
    Updated and included derived files by running: python3 -m script.hassfest.
  • [ ] New or updated dependencies have been added to requirements_all.txt.
    Updated by running python3 -m script.gen_requirements_all.
  • [ ] For the updated dependencies - a link to the changelog, or at minimum a diff between library versions is added to the PR description.
  • [ ] Untested files have been added to .coveragerc.

The integration reached or maintains the following Integration Quality Scale:

  • [ ] No score or internal
  • [ ] 🥈 Silver
  • [ ] 🥇 Gold
  • [ ] 🏆 Platinum

To help with the load of incoming pull requests:

danaues avatar Jul 16 '22 15:07 danaues

Hey there @swails, @bdraco, mind taking a look at this pull request as it has been labeled with an integration (lutron_caseta) you are listed as a code owner for? Thanks! (message by CodeOwnersMention)

@bdraco, can you have a look at this PR again? I think this one is ready. I've added myself as a code owner as you suggested on PR#75323 Thx

danaues avatar Aug 09 '22 20:08 danaues

Is there any way to tell them apart?

bdraco avatar Aug 09 '22 22:08 bdraco

We can query the model to find out. They all have the same leap button numbers for each function, so it works fine as is. The only downside is that the dropdown when making an automation, may have an option that the keypad will never call (ie "raise" for a 4 button keypad)

danaues avatar Aug 09 '22 22:08 danaues

How is the data represented coming back from the bridge on the api?

bdraco avatar Aug 10 '22 01:08 bdraco

This is how the data looks from the api. Device 2171 is a sunnata keypad, 2939 is a Pico

   "2171": {
        "button_groups": ["2180"],
        "current_state": -1,
        "device_id": "2171",
        "fan_speed": None,
        "model": "RRST-W4B-XX",
        "name": "Entry_Entry by Living Room_Fan Keypad_SunnataKeypad",
        "serial": None,
        "type": "SunnataKeypad",
        "zone": None,
    },
    "2939": {
        "button_groups": ["2942"],
        "current_state": -1,
        "device_id": "2939",
        "fan_speed": None,
        "model": "PJ2-3BRL-XXX-A02",
        "name": "Primary Bath_Vanity_Audio Pico_Pico3ButtonRaiseLower",
        "serial": None,
        "type": "Pico3ButtonRaiseLower",
        "zone": None,
    },

danaues avatar Aug 10 '22 01:08 danaues

This is the json that is returned by the leap request to the bridge

{ "CommuniqueType": "ReadResponse", "Header": { "MessageBodyType": "OneDeviceDefinition", "StatusCode": "200 OK", "Url": "/device/2139" }, "Body": { "Device": { "href": "/device/2139", "Name": "Scene Keypad", "Parent": { "href": "/project" }, "ModelNumber": "RRST-W4B-XX", "DeviceType": "SunnataKeypad", "AssociatedArea": { "href": "/area/766" }, "LinkNodes": [ { "href": "/device/2139/linknode/2141" } ], "FirmwareImage": { "href": "/firmwareimage/2139" }, "DeviceClass": { "HexadecimalEncoding": "1270101" }, "AddressedState": "Unaddressed" } } }

danaues avatar Aug 10 '22 01:08 danaues

Can you map "RRST-W4B-XX" to a specific model ?

bdraco avatar Aug 10 '22 01:08 bdraco

Yes, I was originally doing this in pylutron-caseta, but the maintainer only wants to return actual device types, true to the bridges responses. Which I agree with.

if device_model == "RRST-W2B-XX":
    device_type = "SunnataKeypad_2Button"
elif device_model == "RRST-W3RL-XX":
    device_type = "SunnataKeypad_3ButtonRaiseLower"
elif device_model == "RRST-W4B-XX":
    device_type = "SunnataKeypad_4Button"

danaues avatar Aug 10 '22 01:08 danaues

Instead replace this function with something that splits the model and returns the first part.


def _device_model_to_type(model: str) -> str:
    """Convert a lutron_caseta device registry entry model to type."""
    _, device_type = model.split(" ")
    return device_type.replace("(", "").replace(")", "")

Then change all the names to the model fields.

bdraco avatar Aug 10 '22 01:08 bdraco

@bdraco @danaues Any chance this will be useful to help create full RA3 support?

Richard-West avatar Aug 11 '22 01:08 Richard-West

@Richard-West, this is part of the puzzle for full RA3 Support.

The rest is captured in this PR, adding RA3 support to the pylutron-caseta module that this integration depends on. https://github.com/gurumitts/pylutron-caseta/pull/101

We’ve got pairing, lights, shades and Sunnata Keypads working. Still working on Occupancy Sensors, LEDs and scenes. Trying our best to get everything integrated.

There is a discussion here, about getting these things running on your system before it hits mainstream: https://github.com/danaues/pylutron-caseta/issues/2

danaues avatar Aug 11 '22 01:08 danaues

@bdraco , am I on the right page with something like this? Are you ok with the "SunnataKeypad" string literal in there?

https://github.com/danaues/pytesting/blob/7fde6f05083c5e7e8d448e6328ddf08252f24e68/kevin.py#L3-L9

Model String: PJ2-3BRL-GXX-X01 Pico3ButtonRaiseLower Returns: Pico3ButtonRaiseLower

Model String: CS-YJ-4GC-WH FourGroupRemote Returns: FourGroupRemote

Model String: RRST-W4B-XX SunnataKeypad Returns: SunnataKeypad_W4B

danaues avatar Aug 18 '22 14:08 danaues

Or maybe something like this is better. Will match the existing triggers.

https://github.com/danaues/pytesting/blob/36bd36781b8b3069653b4cb2a9bee5ddfe87ae63/kevin2.py#L3-L14

Model String: PJ2-3BRL-GXX-X01 Pico3ButtonRaiseLower Returns: Pico3ButtonRaiseLower

Model String: CS-YJ-4GC-WH FourGroupRemote Returns: FourGroupRemote

Model String: RRST-W4B-XX SunnataKeypad Returns: SunnataKeypad_4Button

danaues avatar Aug 18 '22 15:08 danaues

I'll try to take a look this weekend if it's not too busy.

bdraco avatar Aug 18 '22 16:08 bdraco

We will need to add this to supported brands as well in a future PR

bdraco avatar Aug 18 '22 16:08 bdraco

Do we have a copy of the Lutron LEAP API documentation? Or has the API been reverse engineered from network traces?

Richard-West avatar Aug 18 '22 18:08 Richard-West

Or maybe something like this is better. Will match the existing triggers.

danaues/pytesting@36bd367/kevin2.py#L3-L14

Model String: PJ2-3BRL-GXX-X01 Pico3ButtonRaiseLower Returns: Pico3ButtonRaiseLower

Model String: CS-YJ-4GC-WH FourGroupRemote Returns: FourGroupRemote

Model String: RRST-W4B-XX SunnataKeypad Returns: SunnataKeypad_4Button

We will need to fix the async_validate_trigger_config to map as well if we do it this way

bdraco avatar Aug 18 '22 22:08 bdraco

I merged your suggestion in and fixed the async_validate_trigger_config and the test.

bdraco avatar Aug 18 '22 22:08 bdraco

Testing isn't going so well. I think it needs to be added in init.py during the initial device registry. Otherwise i'm running into issues when retrieving the trigger list, and logging etc.. as the devicetype SunnataKeypad is still present in the device registry.

Back shortly with an alternate plan.

danaues avatar Aug 19 '22 00:08 danaues

Testing isn't going so well. I think it needs to be added in init.py during the initial device registry. Otherwise i'm running into issues when retrieving the trigger list, and logging etc.. as the devicetype SunnataKeypad is still present in the device registry.

Back shortly with an alternate plan.

Make sure you delete it and clean up the deleted devices in device registry manually so it doesn't come back when it sees the same id again

bdraco avatar Aug 19 '22 00:08 bdraco

This is working now. Had to apply the _lutron_model_to_device_type method to device_trigger: async_get_triggers() init: _async_button_event()

danaues avatar Aug 19 '22 01:08 danaues

Thanks @danaues

bdraco avatar Aug 19 '22 02:08 bdraco

Thanks @bdraco

I really appreciate the extra support to get through it!

Cheers

danaues avatar Aug 19 '22 02:08 danaues