gdoor-org / gdoor

Wifi adapter and bus protocol documentation for the Gira TKS Door System
https://gdoor-org.github.io/
GNU General Public License v3.0
14 stars 4 forks source link

Feature/ha discovery #18

Closed DaSchaef closed 1 week ago

DaSchaef commented 1 month ago

untested, may not work

DaSchaef commented 1 month ago

It is now working on my empty, local HA test setup. There is a major change:

GDOOR now sends: "BUS_IDLE" after each bus message, so that the sensor falls back to "BUS_IDLE"

Feedback needed, as I am not a HA user.

jschroeter commented 2 weeks ago

I gave it a try and it works also for me, nice!

Can you please explain why you did the BUS_IDLE change? I do see some downsides of this, e.g. it's harder to read anything meaningful from the HA history view.

history

Further, the HA Logbook is a bit polluted by the BUS_IDLE which I think doesn't add any value?

logbook
DaSchaef commented 2 weeks ago

I implemented BUS_IDLE because otherwise HA shows a wrong state. E.g. the bus is not ringing, but the web interface shows "ringing".

DaSchaef commented 2 weeks ago

Maybe there are other states similar "online"/"offline" we could use? I'm really not into HA :sweat_smile:

nholloh commented 2 weeks ago

I think BUS_IDLE is just fine. The issue is just that with the logbook its quite hard to catch the DOOR_OPEN event, which is barely a second long. I think it might be best to leave it as is for now, since we can find out about the bus data through a serial connection, which is only needed once for setup. You need to establish a serial connection anyway for the initial flash, so the issue shouldn't be too big.

To make it more convenient in the long run, we could add an mqtt topic to set the adapter into some sort of learning mode, which would allow it to record and save the next door open event it captures. You could then use another mqtt topic to directly trigger the door open using the previously recorded bus data. Not sure if this needs to be part of this feature or if we should put it on a separate branch.

I can look into the coding tomorrow. Let me know what you think :)

DaSchaef commented 2 weeks ago

We could just send two MQTT topics?

?

https://community.home-assistant.io/t/mqtt-message-filtering-with-template/42961/2 We could even use templating in the discovery message and announce two entities, where one filters out the BUS_IDLE. No code change in the ESP needed besides the discovery message?

DaSchaef commented 2 weeks ago

To make it more convenient in the long run, we could add an mqtt topic to set the adapter into some sort of learning mode, which would allow it to record and save the next door open event it captures. You could then use another mqtt topic to directly trigger the door open using the previously recorded bus data. Not sure if this needs to be part of this feature or if we should put it on a separate branch.

I would say it would need a new PR (Changelog, Discussion, Tracking, Bugs etc is easier). During the HA coding I also thought of this, but was not sure:

  • How many Door Openers to learn? One? Two?
  • What is with other events, like e.g. Light Button?
  • How to represent it? Button? Sensor?

My problem was the lacl of a good idea about usability.

sim-san commented 2 weeks ago

We could just send two MQTT topics?

  • status: The behavior of this PR, with BUS_IDLE
  • last_message: The behavior of the current main branch, without BUS_IDLE

?

https://community.home-assistant.io/t/mqtt-message-filtering-with-template/42961/2 We could even use templating in the discovery message and announce two entities, where one filters out the BUS_IDLE. No code change in the ESP needed besides the discovery message?

I like the idea of filtering the Messages. So we have one ground truth and different views on this data.

The last 'last_message' could be an entity of the entity category 'diagnostics'. So it will not be added to the dashboard automatically but will still show up in the device page in a card for the category.

See: https://www.home-assistant.io/integrations/sensor.mqtt/#entity_category 16

jschroeter commented 2 weeks ago

I do understand that it's a bit weird that a previously action stays visible until the next occurs but I still think it's better without BUS_IDLE. HA shows when the value was last updated, so you see when the action happened. It's basically the same as with other sensors, e.g. I have an outdoor temperature sensor which only updates once per hour to save battery. The UI also shows an "old" value between the updates, but I can see when it was last updated. Afaik there is no real "event" thing in HA and we have to use a sensor for it?

What exact value to the user does BUS_IDLE bring? I don't really get it yet :-) For me it's rather annoying, as said when looking into the logbook, history and for automations, which will generally trigger twice (action + BUS_IDLE; but probably no big issue since usually you'd filter for specific actions/parameters anyway). The online/offline case should be covered already I think.

The issue is just that with the logbook its quite hard to catch the DOOR_OPEN event, which is barely a second long.

If there is no BUS_IDLE, it's easy to see those in the logbook and sensor attributes?!

...since we can find out about the bus data through a serial connection, which is only needed once for setup. You need to establish a serial connection anyway for the initial flash, so the issue shouldn't be too big.

True, and the web flasher does include a serial terminal where you can see it directly (no need to open a terminal). We could promote this instead of e.g. the HA logbook.

DaSchaef commented 2 weeks ago

You HA user need to comment/decide.

I also found this: https://www.home-assistant.io/integrations/event.mqtt/

nholloh commented 2 weeks ago

I created a separate issue to how to integrate deeper into home assistant, e.g. by using esphome: https://github.com/gdoor-org/gdoor/issues/25. I believe with esphome we could more easily address some of the conceptual challenges we are facing here. Needs some validation with openHAB users, though, because any solution we come up with ultimately should work for them as well.

Also, another issue for learning bus messages: https://github.com/gdoor-org/gdoor/issues/26

nholloh commented 2 weeks ago

To discuss bus_idle I created a separate issue here: https://github.com/gdoor-org/gdoor/issues/27

jschroeter commented 1 week ago

I guess we could merge and close this, it's not ideal yet but it works. And improvements are on their way :-)