rsnodgrass / hass-poolmath

Pool Math for Home Assistant
Other
30 stars 9 forks source link

For each sensor it should look back to find the last value. #4

Closed Gibby closed 4 years ago

Gibby commented 4 years ago

If I create a log entry with all values everything works great.

If say I update just pH, FC and temp. The other sensors stop working.

rsnodgrass commented 4 years ago

Yes. This only returns values for the latest log entry.

I supposed we could add a setting to try to read back through last few log entries and merge them.

Ryan

On Mar 30, 2020 at 7:48 PM, <Gibby (mailto:notifications@github.com)> wrote:

If I create a log entry with all values everything works great.

If say I update just pH, FC and temp. The other sensors stop working.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub (https://github.com/rsnodgrass/hass-poolmath/issues/4), or unsubscribe (https://github.com/notifications/unsubscribe-auth/AAQY4XAI7MGTYIUSWGHND3LRKFKWTANCNFSM4LXFTBEQ).

Gibby commented 4 years ago

Also shouldn't it save the state of the last known good value?

rsnodgrass commented 4 years ago

From reading through the code, it looks like sensor._update_sensors() should only be updating the sensor IF Pool Math returns a value for that state. However, on restart it doesn't have any history of the states for sensors, and so when Pool Math is queried, only the latest values are returned for the last log entry.

Were you seeing this only after restarts or always when Pool Math log entries were created?

Ryan

On Mon, Mar 30, 2020 at 8:24 PM Gibby notifications@github.com wrote:

Also shouldn't it save the state of the last known good value?

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/rsnodgrass/hass-poolmath/issues/4#issuecomment-606376108, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAQY4XEF2C7IRLQVMW4TENTRKFO6PANCNFSM4LXFTBEQ .

Gibby commented 4 years ago

So I only had 2 sensors when I added this yesterday as I had only tested FC and checked temp. I then add a log entry with all results and everything was fine. Today I took just a FC test and after a restart I only had FC and CSI.

rsnodgrass commented 4 years ago

Ok, this behavior makes sense and is the current expected behavior for Home Assistant.

  1. Home Assistant doesn't persist the last sensor value across restarts (the recorded history is different from its current state mechanism)
  2. Home Assistant doesn't persist the knowledge or existence of dynamically generated sensors across restarts
  3. Pool Math only returns the last log entry
  4. hass-poolmath dynamically creates sensors based on what Pool Math returns...if it only returns FC and CSI, then those are the only sensors that are created (until the next update)

So there are several things that cause old sensors/sensor values to disappear after restarts if the full set of sensor values are not saved in each Pool Math log entry.

I'll look into whether we can query Pool Math for the last several log entries, and then merge to create all the sensors and assign values. The downside of this is if Pool Math service is down on a restart of Home Assistant, the sensors won't appear until Pool Math service is restored. I dug around and found a few sensors create their own state files in your /config directory to preserve last sensor values across restarts...but that seems messy if every sensor starting doing that. It seems like Home Assistant should have a mechanism built in to persist both last values and existence of dynamic sensors. I'll dig around a bit more and see if I can find any other possibility before implementing this in the hass-poolmath component.

On Mon, Mar 30, 2020 at 9:04 PM Gibby notifications@github.com wrote:

So I only had 2 sensors when I added this yesterday as I had only tested FC and checked temp. I then add a log entry with all results and everything was fine. Today I took just a FC test and after a restart I only had FC and CSI.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/rsnodgrass/hass-poolmath/issues/4#issuecomment-606386327, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAQY4XF4NIAKII55UBY4QLTRKFTWLANCNFSM4LXFTBEQ .

rsnodgrass commented 4 years ago

Confirmed that Home Assistant's current architecture does not persist most recent sensor values across restarts. So will keep this behavior until HA provides a feature for restoring states after restarts.

HOWEVER, what is bad, is that we should be reading backwards in the Pool Math log entries and merging all the known sensors so they are dynamically created. Otherwise, sensors will not be created that the user is tracking (since hass-poolmath doesn't require users to create a bunch of yaml config to pre-define the sensors). Looking into this.

Gibby commented 4 years ago

Would this work to restore the entity prior to checking for an update after restarts?

https://aarongodfrey.dev/programming/restoring-an-entity-in-home-assistant/

rsnodgrass commented 4 years ago

@gibby Thanks, will dig into RestoreEntity and see if this will resolve the restart issue. Thanks for the pointer!

I also just released 0.0.3 of hass-poolmath that extracts from Pool Math all sensor values as far back as they return, so this fixes the first issue of sensors disappearing if the latest log entry doesn't have all sensors on a reboot (or first time installation). Also some performance improvements and when you click on a sensor value now in Lovelace it will show you what log entry (by timestamp) the value is from.

rsnodgrass commented 4 years ago

@Gibby Update to the latest version (0.0.3) and see how that works out for you. I think this should be sufficient for now.

I also implemented RestoreEntity support (but did not release...leaving it only in master). Since all the sensors are dynamically created from a call to the Pool Math service AND that dynamic creation also always includes the latest log entry data, RestoreEntity support really does nothing useful right now as it tries to restore the previous state AFTER the latest state is already fetched. As long as Pool Math's service is available on startup, all sensors in prior log entries will appear on restarts. In the rare case where the Pool Math cloud service is down exactly when a restart is happening, then sensors won't appear until the next update interval after the Pool Math service is online again.

I'm going to leave the RestoreEntity code in place as a placeholder, even though it isn't useful for dynamic sensors on service outages. If someone wants to implement a solution for this edge case, I added some thoughts at the bottom of README how to use RestoreEntity on the master sensor entity to save previously detected dynamic sensor names. I have too many other high priority features that need implementing in other packages, so I have to skip this.

Gibby commented 4 years ago

@rsnodgrass sweet, just updated and it pulled in values from different logs. Thanks