Conversation
tpanajott
left a comment
tpanajott
left a comment
There was a problem hiding this comment.
Thank you for this PR. Really good work. One thing I can see going wrong here is if the panel as lost connection with MQTT/MQTTManager. If a person then toggles a relay that is default "ON" to be OFF as to turn the light off, when the panel then connects to MQTT again it's not possible to activate the relay again without going through Home Assistant/OpenHAB. One possible solution is to keep track if a relay has changed from its default state using fallback and if we are connected to MQTT/MQTTManager the next time the button is pressed then we toggle the relay back to it's default mode.
|
|
||
|
|
||
| void ButtonManager::_button1_longpress(void) { | ||
| ESP_LOGW("ButtonManager", "button1 longpress"); |
There was a problem hiding this comment.
This should probably be removed. I'm guessing it was used for debugging purposes.
| } | ||
|
|
||
| void ButtonManager::_button2_longpress(void) { | ||
| ESP_LOGW("ButtonManager", "button2 longpress"); |
There was a problem hiding this comment.
This should probably be removed. I'm guessing it was used for debugging purposes.
| size_t packed_data_size = nspanel_mqttmanager_command__pack(&command, buffer.data()); | ||
| if (packed_data_size == packed_length) [[likely]] { | ||
| if (MqttManager::publish(NSPM_ConfigManager::get_manager_command_topic(), (const char *)buffer.data(), packed_length, false) != ESP_OK) [[unlikely]] { | ||
| if (fallback_mode == 0){ |
There was a problem hiding this comment.
What is the purpose of fallback_mode? If it's just used to turn on/off the feature of having a fallback function then it should probably be a boolean.
There was a problem hiding this comment.
I've now had a look in the protobuf and I don't really see the need for fallback_mode being anything other than a boolean, something like fallback_mode_enabled. If we keep this as an enum we should at lease use it to perform what it is meant to do. So, in this check we should do something like fallback_mode != NSPanelManager__FallbackMode::DISABLED.
| if (ButtonManager::_button2_fallback_mode == 0){ | ||
| ESP_LOGE("ButtonManager", "Failed to publish command that button2 was pressed."); | ||
| } else{ | ||
| ESP_LOGE("ButtonManager", "Failed to publish command that button2 was pressed. Fallback toggle relay"); |
There was a problem hiding this comment.
We should probably check that the manager is alive not only that the MQTT message fired properly.
2 participants
this PR has a companion PR in NSPanelManager: https://github.com/vvvrrooomm/NSPanelManager/tree/feat_add_fallback
This adds a fallback mode to the each button action. In case MQTT is not available fallback can be configured to toggle a relays. This basically enables emergency operation if the home automation is disrupted