gluap / pyduofern-hacs

Repository managing hacs-integration for pyduofern
MIT License
27 stars 8 forks source link

[Feature Request] Make five minutes auto update feature configurable (allow enable/disable by user) #26

Closed bcutter closed 1 year ago

bcutter commented 1 year ago

I'd love to see the new https://github.com/gluap/pyduofern-hacs/releases/tag/v0.4.1 Add a low-key auto update by letting the devices schedule an update themselves every five minutes when duofern asks for their state feature a configurable one.

Maybe by using two services to enable/disable it:

  1. This (or any other) way users can benefit from other changes in the newer versions and specifically decide on their own if they prefer to use this feature or not (and e. g. use the duofern.ask_for_update service along with certain entities in case states are not reported reliably).
  2. I fear constant RF noise as I'm using duoFern with plenty of devices. Being able to enable and disable the above mentioned feature would also allow me to check if it really solves the randomly missing states issue.

(Initial thoughts: https://github.com/gluap/pyduofern-hacs/issues/24#issuecomment-1423775805)

8OND007 commented 1 year ago

don't know if it is related to the new 0.4.1 version or not. But since yesterday I sometimes get Unknown msg in my logs:

Logger: /usr/local/lib/python3.10/site-packages/pyduofern/duofern.py Source: /usr/local/lib/python3.10/site-packages/pyduofern/duofern.py:709 First occurred: 07:58:13 (7 occurrences) Last logged: 09:26:41

Unknown msg: 0000407f0e6f12340181000000000000000000000000 Unknown msg: 6f123401810000000000000000000000000000000000 Unknown msg: 004100120000407f0e6f123401810000000000000000 Unknown msg: 00407f0e6f1234018100000000000000000000000000 Unknown msg: 0000640000004100120000407f0e6f12340181000000

I only have 1 Rademacher RolloTron Standard DuoFern 1400 and 1 Rademacher Sonnensensor 9478. Maybe the solar sensor internal battery is recharged again with rescent sunny days and the threshold is reached for sun on/off. Is there a decryption tool somewhere to decode the duofern's msg's ?

Is above a bug, it looks like the message contains the same codes, but is shifted left or right.

bcutter commented 1 year ago

Please check the other issues or create a separate issue as this is not related to this feature request at all.

bcutter commented 1 year ago

I'd love to see the new https://github.com/gluap/pyduofern-hacs/releases/tag/v0.4.1 Add a low-key auto update by letting the devices schedule an update themselves every five minutes when duofern asks for their state feature a configurable one.

Maybe by using two services to enable/disable it:

  • duofern.enable_constant_state_check

  • duofern.disable_constant_state_check

  1. This (or any other) way users can benefit from other changes in the newer versions and specifically decide on their own if they prefer to use this feature or not (and e. g. use the duofern.ask_for_update service along with certain entities in case states are not reported reliably).

  2. I fear constant RF noise as I'm using duoFern with plenty of devices. Being able to enable and disable the above mentioned feature would also allow me to check if it really solves the randomly missing states issue.

(Initial thoughts: https://github.com/gluap/pyduofern-hacs/issues/24#issuecomment-1423775805)

Did you think about this @gluap ?

gluap commented 1 year ago

@bcutter I've thought about this and looked at whether it could be easily enabled/disabled via service. So if it indeed is troublesome we could make it configurable. But with that beta out for a month now without substantial reports about interference I guess it's not causing trouble after all?

bcutter commented 1 year ago

Depends on how many downloads that version has since than. For me e. g. I'm not even on that version yet.

I would really really like to be able to have control over this and continue testing all new (beta) versions.

Once you have more than a dozen DuoFern devices, RF noise starts to be a thing... 😀

gluap commented 1 year ago

I don't think github keeps download statistics for the releases.

The messages take about 0.007 seconds to send (at least that's what I see using rtl-sdr to analyze the RF communication) . So I'd think you would need way more than 12 devices for an update for every device every 5 minutes to cause issues. Of course there will be correlation and re-sent messages occasionally, but even assuming a generous 15 messages a device update should not take more than 1/10 of a second of airtime. So for a dozen of devices you would be below 2 seconds and the 15 messages are a gross overstatement, normally 2-3 should suffice.

I'm willing to implement this provided we find an Issue that can be fixed by it, but with no issues reported with the autoupdating so far I suspect that it may have worked for its intended purpose. And purely for analyzing whether it did I see the following as a possible workaround that would require no extra debug function for toggling the update-request functionality to be implemented:

The comparison between "using 5 minute auto update" and "not using 5 minute auto update" can be done by switching between 0.4.0 and 0.4.3 (or "latest" if you feel adventurous for that matter).

Edit to add: for 0.4.3 vs 0.4.0 there's also another change that may possibly make auto update and sending bursts of Duofern commands more reliable: pyduofern now limits message sending to one every 50 miliseconds, to keep the backchannel open for responses even when sending commands for multiple devices at a time.

bcutter commented 1 year ago

Switching between releases always needs a HA Core restart - using a service would not... ;-)

Did I get this wrong maybe? On a random basis every device sends its status every 5 minutes.

For ten devices:

When I started to use the cover.stop_cover service to specifically trigger an update of a single device, after a time I knew (when and) which devices need more "care" while others update quite reliable.

Hammering every five minutes when not needed just feels like it's not the perfect or smartest approach to fix the randomly missing device status updates. As I still don't have a better idea the only thing I ask for is why not providing users the option to decide if they need/want this feature.

gluap commented 1 year ago

The downside is that it will take about 2-3 hours of work on my side - which competes with different projects and family duties on my side. And I have yet to see an argument that "not having this control" poses any problem beyond "I feel that 2880 is a large number".

I would phrase it this way:

Realistically there will be 3 messages per device every 5 minutes, so 6 seconds of radio frequency transmission per device per day. 6 seconds sounds very small to me and unless these 6 seconds really are problematic I don't see why it needs to be toggleable.

I added the "help wanted" flag. I'm not against merging a PR implementing this - I just personally don't see how it will get priority for me to implement any time soon.

bcutter commented 1 year ago

I understand the time resource part. Really.

I (and I'm not the only one, see linked comment) simply don't like this solution (workaround) for the initial problem (missing device status updates) as it is neither smart nor efficient - it's the hammer method.

My former automation based device status update already was "smarter" as it only forced status updates when there's a medium to high chance the device status has actually been changed (and might be missed) taking sensors into account.

The "5 minutes auto update setting" just blindly sends the status, independently of a status change.

If I miss one status update for one device a day (which I don't according to my long-term log on that issue), why should we stoically send 3 messages for every device every 5 minutes all day.

There might be scenarios where 5 minutes are way to often and there might even be some where a guaranteed instant update is needed so 5 minutes are too rare.

As long as we can't find a smarter approach for the core issue, this still just sounds like a classic setting which should be user configurable.

As discussion seems to not make fundamental progress (two opinions and the dev in the end overrules all other opinions 😃), maybe you can at least quickly elaborate

E. g. what about drastically resending the device status once an actual state change happens? Can we discover this? Like "device status changed --> send new status 10 times within 30 seconds". That would be much more tailored to the initial problem, wouldn't it?

gluap commented 1 year ago

As I said: I'm not opposed to the feature. For me the proposed advantages do not motivate putting in the work. I have not seen any missed states since the update was pushed. I have not seen anyone mention missed states following the introduction of automatic polling. For all I know the implementation works and people are concerned about the possibility it might not work without even testing it. Feel free to open a PR with your implementation - of either a customization or a smarter polling strategy.

If you have a better default value I'd also be open to set that one. Even if that better value would be "0" updates (as I said in #24 disabling this would also be OK for me). Why did I chose to make it default to poll? The majority of users expects stuff too "just work". The worst thing that can happen is when you check your shutters remotely and their state is misrepresented and e.g. it rains into a window that you think has a shutter in front of it. The likelihood of that happening is rather minimal when polling the shutters from time to time, but definitely there when not polling and missing messages from time to time. Also most users will also change state manually from the shutters. So there is no way for homeassistant to know when to poll the shutters. Based on that regular polling seemed to be a sensible answer.

bcutter commented 1 year ago

So

  1. "some preapration to allow setting the auto update timeout later." in https://github.com/gluap/pyduofern-hacs/releases/tag/v0.4.3 is still valid?
  2. I understood https://github.com/gluap/pyduofern-hacs/issues/27 right that all that noise was wiped out of the logs (still exists but is simply ignored now)?

Just catching up the latest releases (stopping at 0.4.3 as 0.5.x need a bit more time as the warning states) and stumbled over those two things as direct impact of the auto update feature.

gluap commented 1 year ago

1.) doing the preparation made me realize the amount of work required to do it proper and lie the project aside because of that. 2.) yes. Because it seems people get stuck up on log messages instead of reporting whether their blinds move which I fail to understand. I would love to keep a lot more logs but half the issues I get are "there's a log message and homeassistant presents it like it shouldn't be there". Even if the event is harmless that causes a lot of work. Reducing verbosity seems to be the only way to make people focus on their blinds.

bcutter commented 1 year ago

instead of reporting whether their blinds move

With https://github.com/gluap/pyduofern-hacs/issues/33 I spotted another strange behavior (covers not moving) which might be related to the constant messages in the duoFERN network due to the 5 minutes auto update function.

I'm begging you / someone able with dev skills to make this "feature" user configurable (custom interval including 0/zero which disables it, e. g. with a service).

gluap commented 1 year ago

This was implemented in 0.5.x