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

Fix battery reporting half on some Nimly locks #3465

Merged
merged 1 commit into from
Oct 29, 2024

Conversation

uvNikita
Copy link
Contributor

Proposed change

Add DoublingPowerConfigurationCluster for some nimly locks.
Following the previous PR #3457.

According to this conversation over at Z2M, only Nimly new locks require this fix, not their old EasyTouch/EasyFinger locks: Koenkk/zigbee-herdsman-converters#6940.

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 Oct 27, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 89.44%. Comparing base (263e68c) to head (5346f52).
Report is 2 commits behind head on dev.

Additional details and impacted files
@@           Coverage Diff           @@
##              dev    #3465   +/-   ##
=======================================
  Coverage   89.44%   89.44%           
=======================================
  Files         311      311           
  Lines       10031    10033    +2     
=======================================
+ Hits         8972     8974    +2     
  Misses       1059     1059           

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

@uvNikita
Copy link
Contributor Author

@TheJulianJES I aslo would like to add some other attributes exposed by this lock that are already available at Z2M (see code here):

  • last_used_pin_code
  • last_action_source
  • last_action_user
  • last_lock_user
  • last_lock_source
  • last_unlock_user
  • last_unlock_source

Could you give me some pointers on how I can implement this? From what I understand looking at Z2M code, the information can be parsed from attribute 256 and 257.
Maybe you could point to an example quirk from this module that is doing something similar?

@blauret
Copy link
Contributor

blauret commented Oct 28, 2024

@uvNikita, maybe you can find inspiration in #3456

@uvNikita
Copy link
Contributor Author

@blauret thanks, that might be what I'm looking for!

I think one puzzle piece I'm still missing is how can I modify published sensor to not just publish raw value, but parse the attribute first and extract information similar to Z2M.

@TheJulianJES
Copy link
Collaborator

how can I modify published sensor to not just publish raw value, but parse the attribute first

You can't, yet. We're looking into adding this.

@TheJulianJES TheJulianJES changed the title Add DoublingPowerConfigurationCluster for some nimly locks Fix battery reporting half on some Nimly locks Oct 29, 2024
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!

@TheJulianJES TheJulianJES merged commit 728ee42 into zigpy:dev Oct 29, 2024
9 checks passed
@uvNikita uvNikita deleted the nimly/battery branch October 30, 2024 07:41
@Eriond
Copy link

Eriond commented Nov 7, 2024

Sorry for this notification being just an observation, instead of a solution proposal, but after updating to HA 2024.11.0 as of today, my ZHA wouldn't start unless I commented out the quirk that adds the battery percentage doubling. I figured that I had to let this excellent solution go in favor of having the rest of my home working 🙂
It's kinda dark now most of the day where I live, so controlling all my lights have precedence over correct battery readings 😄
Perhaps You've had a similar experience? I think it has to do with the quirk being detected as doubled or redundant, from this release?

@TheJulianJES
Copy link
Collaborator

@Eriond Please open an issue, tag me there, and attach logs of the issue.

@uvNikita
Copy link
Contributor Author

uvNikita commented Nov 7, 2024

@Eriond it's probably because the original PR which added support for this lock got into the new home assistant release, but not the doubling fix. And your custom quirk conflicts with the new quirk added.

@Eriond
Copy link

Eriond commented Nov 7, 2024

Yes, that was my conclusion too.
I have now commented out the quirk, and so my logs no longer have the error message in them.
Are you still helped by me recreating the fault (and providing you with the relevant error message), or can you just assume that your assumption is right?

Edit:
I tried to restore that quirk (by un-commenting it), but I don't get the error message anymore. Sorry.

@uvNikita
Copy link
Contributor Author

@TheJulianJES is there a way to override an existing quirk in zha-device-handlers with the one from custom_quirks_path?

As of now, I'm getting

zigpy.exceptions.MultipleQuirksMatchException: Multiple matches found for device <Device model='NimlyPRO' manuf='Onesti Products AS' nwk=0xF84D ieee=f4:ce:36:51:39:99:b3:30 is_initialized=True>
....

@uvNikita
Copy link
Contributor Author

@Eriond I managed to override the quirk with the custom one using this workaround:

from zigpy.quirks import DEVICE_REGISTRY
....

# remove upstream quirk from registry
del DEVICE_REGISTRY._registry_v2[(NIMLY, "NimlyPRO")]

# add custom quirk
(
    QuirkBuilder(NIMLY, "NimlyPRO")
    .node_descriptor(NIMLY_LOCK_NODE_DESCRIPTOR)
    .replaces(DoublingPowerConfigurationCluster, endpoint_id=11)
    .add_to_registry()
)

@Eriond
Copy link

Eriond commented Nov 17, 2024

Thanks for investigating this further.
However, changing the quirk according to your instructions above, doesn't have any effect on the battery sensor. It is still at a steady 50 (%) after replacing with fresh batteries a couple of weeks ago.

This is my current card code:

type: entities
entities:
  - entity: lock.ytterdorr_door_lock
    icon: mdi:door-closed-lock
  - type: custom:template-entity-row
    icon: mdi:battery
    name: Battery remaining {{ states('sensor.ytterdorr_battery') }}%
    state: >-
      {% if is_state('lock.ytterdorr_door_lock', 'locked')%} Locked {% else %}
      Open {% endif %}
    secondary: >-
      Last change {{as_timestamp(states.lock.ytterdorr_door_lock.last_changed) |
      timestamp_custom('%X, %a %d %b')}}

@uvNikita
Copy link
Contributor Author

@Eriond
My code above will work only for Nimly Pro lock, so make sure you have that one or adjust the code for your model.

Also, make sure to re-read the value from Manage Zigbee device → DoublingPowerConfigurationCluster → battery_percentage_remaining.
Might also need to click on “Reconfigure” button for the device.

@Eriond
Copy link

Eriond commented Nov 17, 2024

Yep, that was the trick! Re-reading the value fixed it. Thank you so much.
I thought that restarting the whole machine on which HA is running, should be enough, but apparently not.
Excellent work. I hope it will find it's way into the regular releases.

@gustavostmo
Copy link

@TheJulianJES I am also very interested in getting the same functionality as z2m as @uvNikita wrote higher up. I don't have much experience with Quirk so I can't do it myself unfortunately

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.

5 participants