search

nielsfaber / alarmo

Easy to use alarm system integration for Home Assistant
1.24k stars 117 forks source link

Add History backend (#642) #960

Open f18m opened 3 weeks ago

f18m commented 3 weeks ago

This PR is adding a basic implementation of an HistoryHandler inside Alarmo that is capable of storing all events inside a dedicated sqlite3 database. The implementation is designed to be future-proof: a schema version is added in the database to handle future changes to the database format in a backward-compat manner. The implementation includes a 'purge logic' to make sure a bounded number of events is stored in the database so it does not explode in size. A function to query the database for all events within a time window is provided (this should be used by a frontend ideally). The entire implementation is 212 lines in total (I tried to stay simple&lean).

I tested this PR using the devcontainer methodology and a fake sensor as described in #642 .

Example data produced and stored inside my sqlite3 database:

$ sqlite3 alarmo_events.db  'select * from AlarmoHistoricalEvents;'
1|1717626036.53996|disarm|1717450552|[]
2|1717626037.28198|failed_to_arm|1717450552|{"binary_sensor.fake_sensor1": "open"}
3|1717626037.64322|failed_to_arm|1717450552|{"binary_sensor.fake_sensor1": "open"}
4|1717626041.55363|failed_to_arm|1717450552|{"binary_sensor.fake_sensor1": "open"}
5|1717626047.81397|arm|1717450552|[]
6|1717626052.17818|trigger|1717450552|{"binary_sensor.fake_sensor1": "open"}
7|1717626055.20888|disarm|1717450552|[]

NOTE: the commits of this PR should be squashed before merging.

rolfvreijdenberger commented 3 weeks ago

sounds nice @f18m , but why is this not integrated with the database of choice (the recorder) as configured by the user. I do not want to have my history spread out over multiple database backends (in my case, using mariadb)

f18m commented 3 weeks ago

sounds nice @f18m , but why is this not integrated with the database of choice (the recorder) as configured by the user. I do not want to have my history spread out over multiple database backends (in my case, using mariadb)

hi @rolfvreijdenberger thanks. The reasons why it's not integrated in the Recorder are mainly 2:

  1. to my understanding / investigation on HomeAssistant source code, it's not easy to use the Recorder "overlay" (Python data structures and abstractions) to query by any random attribute. For example in Alarmo it might be useful to build a UI that allows you to search all events associated with a specific "sensor name". Or search all types of events "trigger". I might be wrong but I don't think the Recorder implementation in HA is really meant to be used by other integrations like Alarmo. I think it's heavily coupled with and plumbed into the HA core.
  2. given the sensitivity and the importance of alarm events, I think it could be very useful to have different retention times between the HA Recorder and the Alarmo event databases. Indeed I have implemented a "purge" functionality but I guess it would be useful to support a special value "keep history forever" for the Alarmo event database/registry (given the lean data model, I think we could pack thousands of events into few MBs).
rolfvreijdenberger commented 3 weeks ago

I understand what you're saying in point 2 and simply playing devil's advocate here: what's the forensic use case that justifies keeping it for that long? You're already a bit late responding right :)

also, I can already search in the logbook on events (see screenshot) although only on 1 entity, and not in an overview of related issues. But what is exactly different? image

Maybe I do not quite understand it, I'm not attacking you, but if the use case is to be able to search on related historical events, then that would also be a nice 'more generic' improvement in the logbook/history functionality (and indeed a much bigger change so might be hard to get there) of HA instead of a special case in the alarmo system.

rolfvreijdenberger commented 3 weeks ago

I'll read #642 for context first. Without knowing what's in there fully right now, can't you solve some issues using the actions section with a custom logging rule? in other words, solve it in the alarmo application as an end user implementation instead? logging these events with more contextual data in it seems possible so you add least solve some of the problems stated in #642

f18m commented 3 weeks ago

I'll read #642 for context first. Without knowing what's in there fully right now, can't you solve some issues using the actions section with a custom logging rule? in other words, solve it in the alarmo application as an end user implementation instead? logging these events with more contextual data in it seems possible so you add least solve some of the problems stated in #642

hi @rolfvreijdenberger , my personal use case (mentioned in #642) is that I have some "flaky" alarm sensors and I get false alarms. So I want to have an history of all alarms triggered and check which sensor has triggered the alarm each time. This way I can focus my attention on the most problematic sensors.

This is hard to do using the Recorder alone: I have more than 20 sensors: I should open their recorded statuses together with the Alarmo control panel status to check which sensor had a state transition (closed->open or no-motion->motion) WHILE the alarm was active. Doable but very unconfortable.

And yes I might be building using Alarmo notifications/actions my own "logging system". Indeed for the past few months I've been running my own "logging" system: every time the alarm is triggered I send myself an email with the indication of which sensor triggered it. Honestly however the "history" of the alarm is a basic feature that any alarm system (I'm thinking to traditional smart and non-smart alarms implemented without HA) has. So I think getting a nice UI around it in Alarmo would just be very cool, don't you think so? Moreover I feel like it's doable with a very limited amount of code (the backend I proposed is roughly 200 lines with blanks and comments).

if the use case is to be able to search on related historical events, then that would also be a nice 'more generic' improvement in the logbook/history functionality (and indeed a much bigger change so might be hard to get there) of HA instead of a special case in the alarmo system.

Indeed I think that, if one day HomeAssistant Recorder gains such feature (ability to search on 'related' historical events and conditions), then this History feature of Alarmo might be discarded. However I'm not aware of any future plan for the HomeAssistant Recorder and changing it sounds some work that only HA core developers can do?

nielsfaber commented 3 weeks ago

Hi @f18m Thanks a lot for the PR, I was concerned this feature would become very difficult but the code looks quite lean and elegant! Some thoughts/to-do's I had in mind:

  1. I see that now only alarmo events are subscribed, I would suggest that any state change of an alarm entity or sensor must be logged (and ideally we should also find a way to link a sensor change to triggering of alarm etc.) and ideally also the triggering of an automation/action
  2. For the previous item I think we need to change the structure of the database (fields) to be more generic, perhaps we should look at the HA recorder as an example. it will be hard/impossible to change it afterwards so this needs to have a future-proof layout
  3. We should give the user choice whether the feature is enabled (i.e. add a toggle switch for storage of events), implement a way to limit database size (e.g. remove all entries older than 10 days or similar) and delete the database when alarmo is uninstalled
  4. For the frontend a websocket connection should be made to retrieve data from the database (perhaps with start-end dates as input)

I will keep the PR open for a while until it is feature-complete, when I find the time for it I may add some commits (mostly the frontend part). Thanks again for your efforts so far!

nielsfaber commented 3 weeks ago

@rolfvreijdenberger

also, I can already search in the logbook on events (see screenshot) although only on 1 entity, and not in an overview of related issues. But what is exactly different?

We are indeed trying to make something similar to the logbook, but specific to the alarm. I would like to create an overview of all that happens to sensors and alarm, with the HA logbook you cannot see these together at the moment. Also the "is triggered by ..." text in the logbook is not very useful. I would like this to state which sensor triggered the alarm, which user armed/disarmed the alarm, etc. I haven't found any way to influence the text written here (it seems all handled by HA itself).

f18m commented 3 weeks ago

Hi @f18m Thanks a lot for the PR, I was concerned this feature would become very difficult but the code looks quite lean and elegant!

thanks!

Some thoughts/to-do's I had in mind:

  1. I see that now only alarmo events are subscribed, I would suggest that any state change of an alarm entity or sensor must be logged (and ideally we should also find a way to link a sensor change to triggering of alarm etc.) and ideally also the triggering of an automation/action

ok right -- I guess it makes sense to store the sensor status only when the alarm is active though? Also note in my comment https://github.com/nielsfaber/alarmo/pull/960#issue-2336903829 that the event with ID==6 is a 'trigger' event and it reports the 'sensor_open' that triggered it. In that case it's "binary_sensor.fake_sensor1"...

  1. For the previous item I think we need to change the structure of the database (fields) to be more generic, perhaps we should look at the HA recorder as an example. it will be hard/impossible to change it afterwards so this needs to have a future-proof layout

yes I have included a basic schema upgrade procedure but of course, the better we start from minute zero, the better is.

  1. We should give the user choice whether the feature is enabled (i.e. add a toggle switch for storage of events),

Should the configuration for such feature end up inside the existing AlarmoStorage class (and ultimately come from the Alarmo card UI)?

implement a way to limit database size (e.g. remove all entries older than 10 days or similar)

Inside const.py I added a DATABASE_MAX_EVENTS constant for that. it needs to be exposed as configurable parameter somewhere.

and delete the database when alarmo is uninstalled

Good point. I guess we should be using this: https://developers.home-assistant.io/docs/config_entries_index/#removal-of-entries

  1. For the frontend a websocket connection should be made to retrieve data from the database (perhaps with start-end dates as input)

Yeah I have no much experience with websockets. I do have some time to work on that... perhaps if you can hint the general way to follow I can try.

I will keep the PR open for a while until it is feature-complete, when I find the time for it I may add some commits (mostly the frontend part). Thanks again for your efforts so far!

Thanks!

f18m commented 2 weeks ago

hi @nielsfaber , any update? I might help these days if we agree on the points of comment https://github.com/nielsfaber/alarmo/pull/960#issuecomment-2155173486... thanks!

nielsfaber commented 1 week ago

@f18m I just found out that in the logbook page of HA, you can edit the URL to add multiple entities (separated by %2C). Example:

http://homeassistant.local:8123/logbook?entity_id=alarm_control_panel.alarmo%2Cbinary_sensor.motion_entrance%2Cbinary_sensor.motion_living_room%2Cbinary_sensor.back_door%2Cbinary_sensor.frontdoor%2Cbinary_sensor.bedroom_window%2Cbinary_sensor.studyroom_window&start_date=2024-06-22T07%3A00%3A00.000Z&end_date=2024-06-22T10%3A00%3A00.000Z

This link brings me to a logbook page showing a log for both the alarm entity and my security sensors, for the last 3 hours. It gives a clear overview of what happened at what time, it is very easy to trace back what triggered the alarm. It is also possible to modify the start date+time and stop date+time for the log, so it is possible to create an overview of what happened at the time of triggering of the alarm.

Given that this is already possible through HA, I wonder what is the added value of adding a database and storing all such events, it seems a duplicate implementation. My feeling is that it suffices to add a link in the alarmo GUI that fills in the URL with all sensor+alarm entities. A dedicated database and frontend page of course brings more flexibility for customization, but I don't think this is enough reason for the complete implementation.

What are your thoughts about this? What use-cases do you have in mind for the feature?

f18m commented 1 week ago

hi @nielsfaber ,

@f18m I just found out that in the logbook page of HA, you can edit the URL to add multiple entities (separated by %2C). Example:

http://homeassistant.local:8123/logbook?entity_id=alarm_control_panel.alarmo%2Cbinary_sensor.motion_entrance%2Cbinary_sensor.motion_living_room%2Cbinary_sensor.back_door%2Cbinary_sensor.frontdoor%2Cbinary_sensor.bedroom_window%2Cbinary_sensor.studyroom_window&start_date=2024-06-22T07%3A00%3A00.000Z&end_date=2024-06-22T10%3A00%3A00.000Z

This link brings me to a logbook page showing a log for both the alarm entity and my security sensors, for the last 3 hours. It gives a clear overview of what happened at what time, it is very easy to trace back what triggered the alarm. It is also possible to modify the start date+time and stop date+time for the log, so it is possible to create an overview of what happened at the time of triggering of the alarm.

Actually to be honest I had already noticed that all entities that you choose in the logbook appear concatenated in the URL. However I'm not sure that the view obtained in such a way is really a "clear overview". In my case I have about 20 sensors used with Alarmo. Let's say I want to check which sensor triggered the alarm yesterday: I will have a page with 20 lines saying mostly "motion absent" depicted in gray and there will be just one of them with a tiny yellow line "motion detected"; such event for a flaky sensor in my case lasts about 1-2 sec and then the sensor goes back to "motion absent". That's why it appears as a tiny yellow line, which I need to visually search and visually "interpolate" with the status of Alarmo panel. I mean: it's doable but very user-unfriendly in my opionion.

Given that this is already possible through HA, I wonder what is the added value of adding a database and storing all such events, it seems a duplicate implementation.

Well even if the logbook page was enough for a user, there is the problem of data persistence. For an alarm system you might want to back to alarms triggered even months ago right? Imagine leaving the house for an extended period of time (perhaps it's an house you use only in some months of the year). Having the possibility to use

My feeling is that it suffices to add a link in the alarmo GUI that fills in the URL with all sensor+alarm entities. A dedicated database and frontend page of course brings more flexibility for customization, but I don't think this is enough reason for the complete implementation.

What are your thoughts about this? What use-cases do you have in mind for the feature?

My use case as described in https://github.com/nielsfaber/alarmo/pull/960#issuecomment-2154840377 is that I have some "flaky" alarm sensors (after all HomeAssistant is mostly for DIY users right? so I installed most of the sensors myself...) and I get false alarms. So I want to have an history of all alarms triggered and check which sensor has triggered the alarm each time. This way I can focus my attention on the most problematic sensors. I had this feature also in my "non-smart" Alarm System (before I replaced it with HA+Alarmo): sure it was a crappy "UI" showing events in a 40x2 LCD screen but it did show:

I think this is also what other users were highlighting:

So to conclude: I don't think it makes any sense to create a UI similar to the logbook UI. We should be adding what is not already (easily) available: a simple tabular view that provides an overview of the whole alarm system history. In other words a view about the alarm as a "whole" (activated/deactivated/triggered) together with very important (let's say 'forensic-important') information: who (user activating/deactivating), what (sensor that triggered the alarm or blocked alarmo from arming). For this reason storing in the Alarmo database e.g. the status change of a sensor is probably not useful (and that's why I left that out from this PR). If somebody wants to focus on a specific sensor, then the logbook UI is the right tool. It's important to store in the Alarmo database the 'alarm as a whole' status, which is what's not easily available from logbook.

Hopefully I was able to explain my ideas...