blocking on_not_responding cb

esphome/components/modem/automation.h

@@ -10,13 +10,11 @@ namespace esphome {
 namespace modem {
 
 class ModemOnNotRespondingTrigger : public Trigger<> {
+  // not managed by `add_on_state_callback`, because we want to execute the callback
+  // as a single mode script (we have to know when the callback has ended)
  public:
   explicit ModemOnNotRespondingTrigger(ModemComponent *parent) {
-    parent->add_on_state_callback([this, parent](ModemComponentState state) {
+    parent->set_not_responding_cb(static_cast<Trigger<> *>(this));
-      if (!parent->is_failed() && state == ModemComponentState::NOT_RESPONDING) {
-        this->trigger();
-      }
-    });
   }
 };
 

esphome/components/modem/modem_component.cpp

@@ -108,8 +108,6 @@ network::IPAddresses ModemComponent::get_ip_addresses() {
   esp_err_t err = esp_netif_get_ip_info(this->ppp_netif_, &ip);
   if (err != ESP_OK) {
     ESP_LOGV(TAG, "esp_netif_get_ip_info failed: %s", esp_err_to_name(err));
-    // TODO: do something smarter
-    // return false;
   } else {
     addresses[0] = network::IPAddress(&ip.ip);
   }
@@ -117,6 +115,7 @@ network::IPAddresses ModemComponent::get_ip_addresses() {
 }
 
 std::string ModemComponent::get_use_address() const {
+  // not usefull for a modem ?
   if (this->use_address_.empty()) {
     return App.get_name() + ".local";
   }
@@ -225,13 +224,18 @@ void ModemComponent::reset_() {
   }
 
   set_wdt(60);
-  // be sure that the modem is not in CMUX mode
+  if (this->enabled_ && !this->modem_ready()) {
+    // if the esp has rebooted, but the modem not, it is still in cmux mode
+    // So we close cmux.
+    // The drawback is that if the modem is poweroff, those commands will take some time to execute.
     this->dce->set_mode(esp_modem::modem_mode::CMUX_MANUAL_MODE);
     this->dce->set_mode(esp_modem::modem_mode::CMUX_MANUAL_COMMAND);
     this->dce->set_mode(esp_modem::modem_mode::CMUX_MANUAL_EXIT);
-
     if (!this->modem_ready()) {
       ESP_LOGW(TAG, "Modem still not ready after reset");
+    } else {
+      ESP_LOGD(TAG, "Modem exited previous CMUX session");
+    }
   }
   set_wdt(CONFIG_ESP_TASK_WDT_TIMEOUT_S);
 }
@@ -311,29 +315,18 @@ void ModemComponent::loop() {
   switch (this->state_) {
     case ModemComponentState::NOT_RESPONDING:
       if (this->started_) {
-        if (!this->modem_ready()) {
-          set_wdt(60);
-          ESP_LOGD(TAG, "Modem not responding. Trying to recover...");
-
-          // Errors are not checked, because  some commands return FAIL, but make the modem to answer again...
-          ESP_LOGV(TAG, "Forcing cmux manual mode mode");
-          this->dce->set_mode(esp_modem::modem_mode::CMUX_MANUAL_MODE);
-          ESP_LOGV(TAG, "Forcing cmux manual command mode");
-          this->dce->set_mode(esp_modem::modem_mode::CMUX_MANUAL_COMMAND);
-          ESP_LOGV(TAG, "Forcing cmux manual exit mode");
-          this->dce->set_mode(esp_modem::modem_mode::CMUX_MANUAL_EXIT);
         if (this->modem_ready()) {
-            ESP_LOGI(TAG, "Modem is ready");
+          ESP_LOGW(TAG, "Modem recovered");
           this->state_ = ModemComponentState::DISCONNECTED;
         } else {
-            // try to run `on_not_responding` user callback
+          if (this->not_responding_cb_) {
-            this->on_state_callback_.call(this->state_);
+            if (!this->not_responding_cb_->is_action_running()) {
-            if (!this->modem_ready()) {
+              ESP_LOGD(TAG, "not respondig cb start");
-              ESP_LOGW(TAG, "Unable to recover modem. Reseting dte/dce");
+              this->not_responding_cb_->trigger();
-              this->reset_();
             }
+          } else {
+            ESP_LOGW(TAG, "Modem not responding, and no 'on_not_responding' action defined");
           }
-          set_wdt(CONFIG_TASK_WDT_TIMEOUT_S);
         }
       }
       break;
@@ -420,10 +413,7 @@ void ModemComponent::loop() {
   if (this->state_ != last_state) {
     ESP_LOGV(TAG, "State changed: %s -> %s", state_to_string(last_state).c_str(),
              state_to_string(this->state_).c_str());
-    if (this->state_ != ModemComponentState::NOT_RESPONDING) {
-      // NOT_RESPONDING cb is handled separately.
     this->on_state_callback_.call(this->state_);
-    }
 
     last_state = this->state_;
   }

esphome/components/modem/modem_component.h

@@ -4,6 +4,7 @@
 #include "esphome/core/component.h"
 #include "esphome/core/log.h"
 #include "esphome/components/network/util.h"
+#include "esphome/core/automation.h"
 
 #ifdef USE_ESP_IDF
 
@@ -59,6 +60,7 @@ class ModemComponent : public Component {
   void set_model(const std::string &model) {
     this->model_ = this->modem_model_map_.count(model) ? modem_model_map_[model] : ModemModel::UNKNOWN;
   }
+  void set_not_responding_cb(Trigger<> *not_responding_cb) { this->not_responding_cb_ = not_responding_cb; }
   void add_init_at_command(const std::string &cmd) { this->init_at_commands_.push_back(cmd); }
   std::string send_at(const std::string &cmd);
   bool get_imei(std::string &result);
@@ -70,8 +72,6 @@ class ModemComponent : public Component {
 
  protected:
   void reset_();
-  // void close_();
-  // bool linked_();
   gpio_num_t rx_pin_ = gpio_num_t::GPIO_NUM_NC;
   gpio_num_t tx_pin_ = gpio_num_t::GPIO_NUM_NC;
   std::string pin_code_;
@@ -99,6 +99,7 @@ class ModemComponent : public Component {
   void dump_connect_params_();
   std::string use_address_;
   uint32_t command_delay_ = 500;
+  Trigger<> *not_responding_cb_{nullptr};
   CallbackManager<void(ModemComponentState)> on_state_callback_;
 };