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/wifi #10

Closed DaSchaef closed 1 month ago

DaSchaef commented 2 months ago

Draft pull request for code discussions :) Not finished yet, untested so far.

DaSchaef commented 2 months ago

@ravenst0ne @jschroeter I think it can be tested now. Currently I have no bus setup here, so my tests were limited so far.

Default WiFi Login:

#define DEFAULT_WIFI_SSID     "GDOOR"
#define DEFAULT_WIFI_PASSWORD "12345678"

Rest should be self-explaining I hope (captive portal, setup etc)

ravenst0ne commented 2 months ago

One more finding: The HTML title is the same as the header on the page, so right now we have the base64 encoded image twice, but as title it's not possible to decode it. There is already a todo marked in the Wifimanager library to add supoort to change the title separately (https://github.com/tzapu/WiFiManager/blob/e978bc059c522404c01e06cd136fcf23234eb784/WiFiManager.cpp#L1336)

One workaround would be to add a custom response handler, or better wait until the libary supports it?

DaSchaef commented 2 months ago

One more finding: The HTML title is the same as the header on the page, so right now we have the base64 encoded image twice, but as title it's not possible to decode it. There is already a todo marked in the Wifimanager library to add supoort to change the title separately (https://github.com/tzapu/WiFiManager/blob/e978bc059c522404c01e06cd136fcf23234eb784/WiFiManager.cpp#L1336)

One workaround would be to add a custom response handler, or better wait until the library supports it?

The "WifiManager" Library has even more bugs I worked around, I noticed the title myself, but think it is better to keep as-is and maybe patch it on our own in the future. (it is "only" ugly) - I would not count on them.

jschroeter commented 2 months ago

Would it be possible to add optional MQTT username + password? Wanted to try it with Home Assistant but there the default MQTT config requires authentication.

jschroeter commented 2 months ago

Thanks, I can confirm that the MQTT username/password work.

What I initially didn't realise is that I do need to restart the adapter after changing the MQTT config. Could this be updated without a restart? If not, could we add a hint or maybe even a restart button in the UI?

DaSchaef commented 2 months ago

Thanks, I can confirm that the MQTT username/password work.

What I initially didn't realise is that I do need to restart the adapter after changing the MQTT config. Could this be updated without a restart? If not, could we add a hint or maybe even a restart button in the UI?

It should restart: https://github.com/gdoor-org/gdoor/blob/feature/wifi/firmware/esp32/gdoor/src/wifi_helper.cpp#L319 I need to check tomorrow, if there is a bug.

jschroeter commented 2 months ago

You're right, it does. Just tried it again and it worked. Not sure why it didn't work for me before.

jschroeter commented 2 months ago

I'm currently observing something weird which happened twice in the last days:

DaSchaef commented 2 months ago

Very strange, the mentioned behavior does not align to what I would expect on wifi disconnect. Can you reproduce it with wifi on/off?

jschroeter commented 2 months ago

Some more observations since it happened again:

Initially I thought it's an issue with the Home Assistant integration, but it seems unlikely that serial and MQTT break at the same time and wouldn't explain why my router showed the adapter as disconnected from the wifi.

DaSchaef commented 2 months ago

Is the observation via homeassistent or also via e.g. mqttbrowser and a serial console? We really need to exclude homeassistent as error source. The symptoms are very very strange.

I'm currently preparing a build with more debug output.

DaSchaef commented 2 months ago

I added more debug outputs (untested, maybe I broke it).

Please log the serial debug prints, it is very verbose,, but maybe we can see what happens. My only idea would be that the MQTT sending is somehow blocking

Hmmm, if something went wrong with MQTT/WIFI, maybe the MQTT_PRINTER Buffer was too full? I added big fat debug print statement for such a condition.

jschroeter commented 1 month ago

Good idea trying it with MQTT Explorer! I think I found the issue and it's related to Home Assistant. Apparently HA gets confused when sensor values change types, e.g. from valid JSON, to invalid JSON, to string etc. This is true for the serial integration but also for MQTT. And sometimes the sensor doesn't recover, it keeps showing the value "unknown" even if it's a valid JSON again. For serial it's also important to always terminate with a new line which was missing in two places (017ccee).

In general, I think it makes sense to have a consistent output. Let's discuss this in #15. And I would say we can merge this PR.