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 IKEA Rodret dimmer support #2609

Merged
merged 6 commits into from Nov 1, 2023

Conversation

inventorofthingies
Copy link
Contributor

Proposed change

Added signature to match Ikea's Rodret dimmer button.

Additional information

Rodret is basically same as Tradfri without alarm input cluster and without window covering output cluster. Tested as a standalone quirk

Checklist

  • The changes are tested and work correctly
  • pre-commit checks pass / the code has been formatted using Black
  • Tests have been added to verify that the new code works

Add support for Rodret dimmers
Make comment match
@TheJulianJES TheJulianJES added the new quirk Adds support for a new device label Sep 28, 2023
@codecov
Copy link

codecov bot commented Sep 28, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (7385465) 86.57% compared to head (e0233c3) 86.58%.
Report is 22 commits behind head on dev.

❗ Current head e0233c3 differs from pull request most recent head b736f27. Consider uploading reports for the commit b736f27 to get more accurate results

Additional details and impacted files
@@           Coverage Diff           @@
##              dev    #2609   +/-   ##
=======================================
  Coverage   86.57%   86.58%           
=======================================
  Files         277      277           
  Lines        8523     8527    +4     
=======================================
+ Hits         7379     7383    +4     
  Misses       1144     1144           
Files Coverage Δ
zhaquirks/ikea/twobtnremote.py 100.00% <100.00%> (ø)

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

zhaquirks/ikea/twobtnremote.py Outdated Show resolved Hide resolved
@@ -195,3 +195,63 @@ class IkeaTradfriRemote2BtnZLL(CustomDevice):
}

device_automation_triggers = IkeaTradfriRemote2Btn.device_automation_triggers.copy()

class IkeaRodretRemote2Btn(CustomDevice):
"""Custom device representing IKEA of Sweden RODRET remote control."""
Copy link
Contributor

Choose a reason for hiding this comment

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

Better description of the device then its 2 more upcoming Shortcut and OpenClose and the first is CSA certed for some weeks ago.

Copy link
Contributor

@MattWestb MattWestb Sep 28, 2023

Choose a reason for hiding this comment

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

SOMRIG shortcut button Aka. E2213

https://csa-iot.org/csa_product/somrig-shortcut-button/

OpenCloce is coming then IKEA have using all produces hardware they have in storage at the factory's (and they need do new packages for all blinds).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've not seen a SOMRIG yet -- happy to have a go at adding when I get one. Let me know how to handle different firmware variants and I'll add that later too.

Copy link
Contributor

Choose a reason for hiding this comment

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

I have not seen it in one store but its very like weeks and not months before its start pooing up little here and there.
Its very likely its using Matter commands on one private cluster then its not fixed in Zigbee (next ZCL is command with Matter) like they have on Symfonisk Gen 2 scene scene buttons but we will see then we is getting them.

By the way good work done !!!

Copy link
Collaborator

Choose a reason for hiding this comment

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

Better description of the device then its 2 more upcoming Shortcut and OpenClose and the first is CSA certed for some weeks ago.

Is this a request about something that should be changed? IMO, class name and comment are fine.

Choose a reason for hiding this comment

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

Thank MattWestb
I dindt quite understand everything you said, since i am sort of a newbie :) But you made me hopefull again haha.
So the SOMRig is not using the same hardware protocls that Rodret i assume. Even togh they seem identical..

Copy link
Contributor

Choose a reason for hiding this comment

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

Hardware is identical only the printing on it is different and the firmware is different and sending commands on one different cluster that is not standard and need one quirk for fixing it,

Choose a reason for hiding this comment

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

Ok. Thank you a lot. looking forward to your quirk :)

Choose a reason for hiding this comment

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

Any news? I would also donate you something.
Cheers

Copy link
Contributor

Choose a reason for hiding this comment

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

Im on one 2 weeks trip and have only laptop with and no connection to my test networks so i cant do any testings but if some one like coding and testing i can helping with little knowledge that i was taking with ;-)

DoublingPowerConfig1CRCluster,
PowerConfiguration.cluster_id,
Copy link
Collaborator

Choose a reason for hiding this comment

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

Don't seems related to your quirk. It is needed?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Yeah, let's not change the old two button dimmer in this PR.

The problem is that newer IKEA firmware versions didn't change the device signature for the (old) two button dimmer.
We shouldn't double battery percentage on those newer firmware versions, but still on old firmware. But we can't match by firmware version at the moment.

The signature also isn't "refreshed" when just updating the firmware through ZHA. I think it needs to be removed from ZHA and completely re-added. (I don't think re-pairing is enough, but I'm not sure right now)


Without matching per firmware version, maybe it's possible to find out what voltage 0% and 100% respond to on IKEA remotes. @MattWestb Any idea? 😄
At least for the two button dimmer, we could use a linear function to calculate the battery percentage based on the battery voltage (like we do for some other devices). That way it would work regardless of the firmware version -- assuming IKEA sends a proper battery voltage.

Or maybe we just do it the Z2M way and remove doubling everywhere -- regardless of firmware version.

Copy link
Contributor

Choose a reason for hiding this comment

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

My "beloved" d(f)conz have just removing it for all IKEA controllers then they is expecting all user updating the firmware and its one way to going.
But i think its better doing one separate PR for that and doing all but not the old rotating dimmer then its still on 2.3.X firmware and the classes for older ZLL and also ZB3 versions.

If i remember right is most reporting OK battery voltage but first gen Symfonisk is no (i think 25V is full) and perhaps some more but its not easy testing that all my controllers is on latest firmware without j-tag flashing all types them with one older firmware and testing.

Z2M is doing matching on SW_Build for gen 3 controller and i think on the old ones 2.

ZHA is having the SW_build and version saved in the Zigbe.DB so one way is implanting quirk to using it and its all good but is it needed for most users ?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Reverted that unrelated change in cbfb4d5

We still need to address this, but preferably without breaking outdated or updated dimmers.
Since the battery voltage should be reported the same (regardless of if it's updated or not), we should be able to map voltage to percentage manually and just ignore the percentage the device sends.
We really need to find out how the device maps battery voltage to percentage though.
Would be amazing if somebody can give us multiple data points on which voltage corresponds to which percentage.

Copy link
Contributor

Choose a reason for hiding this comment

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

Then you need verifying that all firmware versions is sending correct voltage then i have see dome IKEA controllers sending no or wrong voltage (Symfonisk first gen was sending 25V and not 2.5V in some version).
I think better taking the double for the quirk class that also being loaded by 24.x firmware if have the same as the classes and letting all old have the doubling and user shall updating there devices for fixing battery darning and network loss then running battery low.

DEVICE_TYPE: zha.DeviceType.NON_COLOR_CONTROLLER,
INPUT_CLUSTERS: [
Basic.cluster_id,
PowerConfiguration.cluster_id,
Copy link
Collaborator

Choose a reason for hiding this comment

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

This isn't important, but using a cluster like this (important: non-doubling), it would add information about battery type/quantity/voltage:

class PowerConfig2AAACluster(CustomCluster, PowerConfiguration):
"""Updating power attributes: 2 AAA."""
_CONSTANT_ATTRIBUTES = {
BATTERY_SIZE: 4,
BATTERY_QUANTITY: 2,
BATTERY_RATED_VOLTAGE: 15,
}

At least the battery type should be displayed as an additional attribute when clicking on the battery entity in HA then.

(The cluster above would need to be "duplicated" and quantity changed to 1, as the device only seems to take one AAA battery.)

Copy link
Collaborator

Choose a reason for hiding this comment

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

Addressed in: b736f27

@TheJulianJES TheJulianJES changed the title Add ikea rodret Add IKEA Rodret dimmer support Sep 29, 2023
Copy link
Collaborator

@TheJulianJES TheJulianJES left a comment

Choose a reason for hiding this comment

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

I've pushed two commits to resolve the remaining issues with this and it should be good now. Thanks for the PR!

(This should make it into HA Core 2023.12.x.)

@TheJulianJES TheJulianJES merged commit d1a1b59 into zigpy:dev Nov 1, 2023
12 checks passed
elupus pushed a commit to elupus/zha-device-handlers that referenced this pull request Jan 17, 2024
* Update twobtnremote.py

Add support for Rodret dimmers

* Update twobtnremote.py

Make comment match

* feedback in pullrequest related to adding Rodret

* no need to double power

* Undo unrelated non-doubling change for two button remote

* Add `PowerConfig1AAACluster` for Rodret

---------

Co-authored-by: TheJulianJES <TheJulianJES@users.noreply.github.com>
lgraf pushed a commit to lgraf/zha-device-handlers that referenced this pull request May 6, 2024
* Update twobtnremote.py

Add support for Rodret dimmers

* Update twobtnremote.py

Make comment match

* feedback in pullrequest related to adding Rodret

* no need to double power

* Undo unrelated non-doubling change for two button remote

* Add `PowerConfig1AAACluster` for Rodret

---------

Co-authored-by: TheJulianJES <TheJulianJES@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
new quirk Adds support for a new device
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants