jimpastos / ha-modernforms

ModernForms Smart Fan Integration for HomeAssistant
MIT License
12 stars 6 forks source link

Increase scan interval to 60 secons to address issue #11. #13

Closed icemarkom closed 3 years ago

icemarkom commented 3 years ago

This PR addresses the issue #11 by simply bumping the configured scan_interval to 60 seconds.

I was trying to figure out how to configure a custom scan interval in the config, but for example this didn't work:

configuration.yaml

fan:
- platform: modernforms
  scan_interval: 60
jimpastos commented 3 years ago

Why didnt the config work? Maybe a parsing issuse?

icemarkom commented 3 years ago

I am not sure why parsing didn't work, but here's what happens when I try it:

configuration.yaml:

fan:
- platform: modernforms
  scan_interval: 45

Log:

mff-ha           | 2021-04-17 08:30:16 ERROR (MainThread) [homeassistant.components.fan] The modernforms platform for the fan integration does not support platform setup. Please remove it from your config.

Is, perhaps, the right thing to do:

modernforms:
  scan_interval: 45

It would be nice to document this in readme. I am happy to send PR with the change for that as well (in fact rewrote the whole thing yesterday, and then discovered it was in fact broken).

icemarkom commented 3 years ago

Let me try this again. I ended in git hell. I hat git profusely.

barlock commented 3 years ago

😅 It appears I broke this in an earlier PR. I removed the configuration file and replaced it with a newer GUI flow that, I think, doesn't read from the config file anymore.

I've been running and testing a similar issue and it's interesting that we independently came to an interval of 60 to clean up the problem. Took me a few weeks of testing before I felt comfortable making a PR.

I've done some more refactoring to try and keep the pings even lower, I'll update that flow to also include a scan_interval in the GUI flow so we can raise this value if we need to in the future without a code change.

Super frustrating to have your fan lock up and need a power reset 😭

icemarkom commented 3 years ago

Yup, just use the reboot automation every 3-4 hours.

Also, 60 seconds is a fairly safe stable value for a few days, but it will still lock them up. 60 seconds, combined with a reboot every few hours is a winner. Hence my FR here to just add the reboot automation to the integration and be done with it :-). We can't fix MF's bad code, but we can work around it.