node-zwave-js icon indicating copy to clipboard operation
node-zwave-js copied to clipboard

Automatic Battery CC refresh for non-listening devices

Open kpine opened this issue 3 years ago • 8 comments

Is your feature request related to a problem? Please describe. No.

Describe the solution you'd like Refresh the Battery CC values automatically for non-listening devices based on a time-based schedule (at once a month or more often since last update).

Not all Battery devices send battery reports unsolicited, or maybe the controller was down when a device did send an update, so this would be useful as default functionality. Listening devices already have this behavior.

Describe alternatives you've considered

  • Enable polling or automated updates in automation software. This works fine but requires manual setup.
  • Use undocumented (?) queryOnWakeup compat flag with battery CC. Probably a bit excessive to query on every wake-up. This is enabled on a few battery thermostat devices, for what looks like other purposes.
  • Add a new compat flag specifically for time-based refreshes, instead of per-wake up. Could individually opt-in those devices that don't send unsolicited battery reports.

Additional context Some devices may never wake up on their own, if opt-in by default, would it be necessary to disable for some devices? At most it would keep a pending Get message in the queue.

kpine avatar May 10 '21 16:05 kpine

I've seen a bit of the discussion in Discord. Are the devices in question Z-Wave Plus?

AlCalzone avatar May 10 '21 17:05 AlCalzone

I have a few door/window sensors that are z-wave plus. I've never seen them send battery updates. Maybe they are outliers?

kpine avatar May 10 '21 17:05 kpine

I'm thinking of just removing the check for sleeping/non-sleeping and scheduling the refreshes for every node. When they do send unsolicited updates for a CC, those scheduled refreshes get canceled again, so it shouldn't hurt.

AlCalzone avatar May 10 '21 17:05 AlCalzone

With the original ioBroker Z-Wave adapter our battery levels seemed to be updated so went looking at the https://github.com/ioBroker/ioBroker.zwave / https://github.com/jperkin/node-openzwave code for clues as to why that was. Sadly it's not very obvious :disappointed:

If anyone knows for sure (seems to be a bit of speculation on this issue) why the old adapter & library battery update works versus why the new one doesn't... well... I'm happy to mess about with some tests and have a stab at a PR for this but need pointing in the right direction to get going is all. Thx.

raintonr avatar Jan 22 '22 09:01 raintonr

Poking around more... found issue #203, closed with PR #558, which introduced code that appears to request battery status periodically.

Comment in the code says once a month spec but that's not frequent enough and actually shows every 7 days. Perhaps we should add a debug thingo to make this do every wake-up or something just in test?

That aside, I looked in BatteryCC and there's the refreshValues method that should be triggered by the above.

So what gives - why isn't this working?

raintonr avatar Jan 22 '22 17:01 raintonr

Battery refresh is only done for devices like locks (FLIRS). That's why this feature request was created.

I don't actually think it refreshes batteries for lock devices either. No refresh is done for a Z-Wave Plus device.

kpine avatar Jan 22 '22 17:01 kpine

Well, just to test I copied the stuff from inside the if here... https://github.com/zwave-js/node-zwave-js/blob/f4b8e06b80cd75177d5543ab4cb65a73123dab1b/packages/zwave-js/src/lib/node/Node.ts#L1956

... and put it inside handleWakeUpNotification (after the this.markAsAwake(); call). I also changed the frequency to 10 minutes just to make sure it thought my devices needed refreshing. And yes... that works just fine and got battery levels updated.

Guess that is a sub-optimal fix though, will try and do something smarter as proposed above and make a PR.

raintonr avatar Jan 22 '22 18:01 raintonr

Dependency on #1608 to be Done Right™

... because one needs to know in a persistent way when last updates occurred.

raintonr avatar Feb 09 '22 21:02 raintonr

Closing in favor of #5558 which contains a few more requirements for other CCs aswell

AlCalzone avatar Mar 10 '23 12:03 AlCalzone