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

Remove LevelControl cluster for IKEA INSPELNING plugs #3379

Merged
merged 1 commit into from
Sep 24, 2024

Conversation

folfy
Copy link
Contributor

@folfy folfy commented Sep 22, 2024

The new smart plug suffers from the same issue like all IKEA plugs, unnecessary level controls in the configuration panel:
inspelning

Proposed change

Expanded Quirk to be applied to new plug as well.

Additional information

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

Copy link

codecov bot commented Sep 22, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 88.49%. Comparing base (d9c9df9) to head (c9540c7).
Report is 2 commits behind head on dev.

Additional details and impacted files
@@           Coverage Diff           @@
##              dev    #3379   +/-   ##
=======================================
  Coverage   88.49%   88.49%           
=======================================
  Files         305      305           
  Lines        9621     9621           
=======================================
  Hits         8514     8514           
  Misses       1107     1107           

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

@MattWestb
Copy link
Contributor

Its the "personality" of all IKEA Zigbee 3.0+ devices then they is using remotes with level with On / Off then being direct paired with devices (tush link).
Funny is that Tretakt is also exposing the light level attributes with Dirigera and Matter integration to HA so very likely Inspelning is doing the same.

By the way where did you getting the plug ?
I have not seen it in Vienna but its in there the system but hidden and not in the store shelf.

Copy link
Contributor

@MattWestb MattWestb left a comment

Choose a reason for hiding this comment

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

Looks very OK for me.

@folfy
Copy link
Contributor Author

folfy commented Sep 22, 2024

@MattWestb Yeah, it's not on the website or in their "normal" system yet, but I asked at Wien Westbahnhof on Thursday, and they told me they get a shipment of 30pcs on Friday to be put into the shelves the same day. Sure enough they were there in the afternoon, as promised, so feel free to grab one while they last. :D

Also, there's issue #3374 - Looks like we need to update the ac_power_divisor and consider it on power-value updates, see my last comment

@MattWestb
Copy link
Contributor

I was also at Wbh on Thursday but was not asking and need going on Monday if i can get time for getting some of them.
Its very likely the power is not OK set in the device but also if it the original firmware or the latest production ?
If its only the factory one then leaving it and update the firmware to the latest production one and dont need struggling with quirk.

@MattWestb
Copy link
Contributor

MattWestb commented Sep 22, 2024

Interesting testing you have done !!
I was not knowing it was allowed changing this attribute "on the fly".
First i was thinking they way using wrong datatypes and must being corrected for getting the full range but with your testing i think its one new way of "extending the Zigbee specification" but i think its need being implanted i ZHA /zigpy.

Ping our @puddly and @TheJulianJES !!!
Some thinking of this problem ?
If IKEA is using "dynamic changing attribute" on the Measurement Cluster we must pulling it or catching if getting it reprtebal in the device so the system can using it.

@puddly
Copy link
Contributor

puddly commented Sep 22, 2024

Is this attribute reportable? I wonder how the IKEA hubs avoids polling the attribute.

@folfy
Copy link
Contributor Author

folfy commented Sep 22, 2024

@puddly Unfortunately don't have an IKEA hub, but tomorrow I'll try to check in the ZHA logs if I maybe can see the attribute being pushed by the plug automatically, where it maybe/hopefully just isn't evaluted by us yet. So far just been manually fetching/checking it for the first analysis of the issue:
inspelning_cluster
If not, I'll consider to get some IKEA hub, just to find out what's going on there xP

@puddly
Copy link
Contributor

puddly commented Sep 22, 2024

The attribute is apparently reportable according to the spec, we should just add it to the list of attributes to set up for reporting.

I hope the device sends updates to that attribute first. Otherwise, the quirk will need logic to handle it.

@TheJulianJES TheJulianJES changed the title Quirk for IKEA plugs - added new INSPELNING plug Remove LevelControl cluster for IKEA INSPELNING plugs Sep 22, 2024
@MattWestb
Copy link
Contributor

Thanks our @puddly for taking on look !!
You have getting one email with my first paring with DIRIGERA but its on the factors firmware so perhaps not OK.
I do one new sniff then its updated and sending it to you.

Thanks in advance !!!

@MattWestb
Copy link
Contributor

With the latest firmware is indeed reporting the AC Power Divisor (0x0605) if the power his higher then the lowerst setting and also updating it later if its needed.
But is normally not sending one 0 if the current power is zero in the last report.
So some logic need being implemented here.
Here is one sniff with one toaster with 1300W and one light of 0.5 W and some zero loaded between.

INSPPELNING03.pcapng.gz

(PS: only Puddly have my network key)

@MattWestb
Copy link
Contributor

PS: I think we shall moving the power factor out of this PR then its good ass it it and making one new PR / issue for the "dynamic power factor" implementation.

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.

Thanks!
(The ac_power_divisor attribute stuff needs to be addressed separately in ZHA.)

@TheJulianJES TheJulianJES merged commit d07b489 into zigpy:dev Sep 24, 2024
7 checks passed
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.

4 participants