better sim error checks

esphome/components/modem/modem_component.cpp

@@ -72,31 +72,24 @@ bool ModemComponent::is_modem_connected(bool verbose) {
 AtCommandResult ModemComponent::send_at(const std::string &cmd, uint32_t timeout) {
   AtCommandResult at_command_result;
   at_command_result.success = false;
-  command_result status = command_result::FAIL;
+  at_command_result.esp_modem_command_result = command_result::TIMEOUT;
-  if (this->modem_ready()) {
+  if (this->dce) {
     ESP_LOGV(TAG, "Sending command: %s", cmd.c_str());
-    status = this->dce->at(cmd, at_command_result.result, timeout);
+    at_command_result.esp_modem_command_result = this->dce->at(cmd, at_command_result.output, timeout);
-    ESP_LOGD(TAG, "Result for command %s: %s (status %s)", cmd.c_str(), at_command_result.result.c_str(),
+    ESP_LOGD(TAG, "Result for command %s: %s (status %s)", cmd.c_str(), at_command_result.c_str(),
-             command_result_to_string(status).c_str());
+             command_result_to_string(at_command_result.esp_modem_command_result).c_str());
-  }
-  if (status == command_result::OK) {
-    at_command_result.success = true;
   }
+  at_command_result.success = at_command_result.esp_modem_command_result == command_result::OK;
   return at_command_result;
 }
 
-AtCommandResult ModemComponent::send_at(const std::string &cmd) { return this->send_at(cmd, this->command_delay_); }
-
 AtCommandResult ModemComponent::get_imei() {
   // get the imei, and check the result is a valid imei string
   // (so it can be used to check if the modem is responding correctly (a simple 'AT' cmd is sometime not enough))
   AtCommandResult at_command_result;
-  at_command_result.success = false;
+  at_command_result = this->send_at("AT+CGSN", 4000);
-  command_result status = command_result::FAIL;
+  if (at_command_result.success && at_command_result.output.length() == 15) {
-  status = this->dce->at("AT+CGSN", at_command_result.result, 1000);
+    for (char c : at_command_result.output) {
-  if ((status == command_result::OK) && at_command_result.result.length() == 15) {
-    at_command_result.success = true;
-    for (char c : at_command_result.result) {
       if (!isdigit(static_cast<unsigned char>(c))) {
         at_command_result.success = false;
         break;
@@ -513,6 +506,9 @@ bool ModemComponent::modem_sync_() {
   std::string result;
 
   ESP_LOGV(TAG, "Checking if the modem is synced...");
+
+  this->flush_uart_();
+
   bool status = this->get_imei();
   if (!status) {
     // Try to exit CMUX_MANUAL_DATA or DATA_MODE, if any
@@ -522,12 +518,13 @@ bool ModemComponent::modem_sync_() {
     auto command_mode = [this]() -> bool {
       ESP_LOGVV(TAG, "trying command mode");
       this->dce->set_mode(modem_mode::UNDEF);
-      return this->dce->set_mode(modem_mode::COMMAND_MODE);
+      return this->dce->set_mode(modem_mode::COMMAND_MODE) && this->get_imei();
     };
 
     auto cmux_command_mode = [this]() -> bool {
       ESP_LOGVV(TAG, "trying cmux command mode");
-      return this->dce->set_mode(modem_mode::CMUX_MANUAL_MODE) && this->dce->set_mode(modem_mode::CMUX_MANUAL_COMMAND);
+      return this->dce->set_mode(modem_mode::CMUX_MANUAL_MODE) &&
+             this->dce->set_mode(modem_mode::CMUX_MANUAL_COMMAND) && this->get_imei();
     };
 
     // The cmux state is supposed to be the same before the reboot. But if it has changed (new firwmare), we will try
@@ -546,7 +543,6 @@ bool ModemComponent::modem_sync_() {
       this->internal_state_.powered_on = false;
     } else {
       ESP_LOGD(TAG, "Connected to the modem in %" PRIu32 "ms", elapsed_ms);
-      delay(2000);  // NOLINT
       this->internal_state_.powered_on = true;
     }
   } else {
@@ -567,12 +563,17 @@ bool ModemComponent::modem_sync_() {
       return false;
     }
 
+    this->send_init_at_();
+
+    if (!this->pin_code_.empty()) {
       if (!this->prepare_sim_()) {
         // fatal error
         this->disable();
         status = false;
       }
-    this->send_init_at_();
+    } else {
+      ESP_LOGI(TAG, "No pin_code, so no pin check");
+    }
 
     ESP_LOGI(TAG, "Modem infos:");
     std::string result;
@@ -588,33 +589,35 @@ bool ModemComponent::modem_sync_() {
 }
 
 bool ModemComponent::prepare_sim_() {
-  // it seems that read_pin(pin_ok) unexpectedly fail if no sim card is inserted, whithout updating the 'pin_ok'
+  command_result modem_status;
+
+  std::string output;
+
+  // this->dce->read_pin(pin_ok)   // not used, because we can't know the cause of the error.
+  modem_status = this->dce->command(
+      "AT+CPIN?\r",
+      [&](uint8_t *data, size_t len) {
+        output.assign(reinterpret_cast<char *>(data), len);
+        std::replace(output.begin(), output.end(), '\n', ' ');
+        return command_result::OK;
+      },
+      this->command_delay_);
+
+  ESP_LOGD(TAG, "SIM: %s", output.c_str());
+
+  if (output.find("+CPIN: READY") != std::string::npos) {
+    return true;  // pin not needed or already unlocked
+  } else {
+    if (output.find("SIM not inserted") != std::string::npos) {
+      this->abort_("Sim card not inserted.");
+    }
+  }
+
+  ESPMODEM_ERROR_CHECK(this->dce->set_pin(this->pin_code_), "Set pin error");
+
   bool pin_ok = false;
-  if (this->dce->read_pin(pin_ok) != command_result::OK) {
+  ESPMODEM_ERROR_CHECK(this->dce->read_pin(pin_ok), "Error checking pin");
-    this->status_set_error("Unable to read pin status. Missing SIM card?");
-    return false;
-  }
 
-  delay(this->command_delay_);
-
-  if (!pin_ok) {
-    if (!this->pin_code_.empty()) {
-      ESP_LOGV(TAG, "Set pin code: %s", this->pin_code_.c_str());
-      ESPMODEM_ERROR_CHECK(this->dce->set_pin(this->pin_code_), "Set pin code failed");
-      delay(2000);  // NOLINT
-    }
-  }
-
-  ESPMODEM_ERROR_CHECK(this->dce->read_pin(pin_ok), "Check pin");
-  if (pin_ok) {
-    if (this->pin_code_.empty()) {
-      ESP_LOGD(TAG, "PIN not needed");
-    } else {
-      ESP_LOGD(TAG, "PIN unlocked");
-    }
-  } else {
-    this->status_set_error("Invalid PIN code.");
-  }
   return pin_ok;
 }
 
@@ -626,9 +629,9 @@ void ModemComponent::send_init_at_() {
     if (!at_command_result) {
       ESP_LOGE(TAG, "Error while executing 'init_at' '%s' command", cmd.c_str());
     } else {
-      ESP_LOGI(TAG, "'init_at' '%s' result: %s", cmd.c_str(), at_command_result.result.c_str());
+      ESP_LOGI(TAG, "'init_at' '%s' output: %s", cmd.c_str(), at_command_result.output.c_str());
     }
-    delay(200);  // NOLINT
+    delay(2000);  // NOLINT
   }
 }
 
@@ -762,6 +765,31 @@ void ModemComponent::dump_connect_params_() {
   ESP_LOGCONFIG(TAG, "  DNS fallback: %s", network::IPAddress(dns_fallback_ip).str().c_str());
 }
 
+bool ModemComponent::flush_uart_() {
+  size_t cleaned = 0;
+
+  this->dce->command(
+      "",
+      [&](uint8_t *data, size_t len) {
+        cleaned = len;
+        return command_result::OK;
+      },
+      1000) == command_result::OK;
+
+  if (cleaned != 0) {
+    ESP_LOGW(TAG, "Cleaned %d modem buffer data", cleaned);
+  }
+}
+
+const char *AtCommandResult::c_str() const {
+  if (success) {
+    cached_c_str = output + " (OK)";
+  } else {
+    cached_c_str = output + " (" + command_result_to_string(esp_modem_command_result) + ")";
+  }
+  return cached_c_str.c_str();
+}
+
 }  // namespace modem
 }  // namespace esphome
 

esphome/components/modem/modem_component.h

@@ -43,16 +43,18 @@ enum class ModemPowerState {
 };
 
 struct AtCommandResult {
-  std::string result;
+  std::string output{};
-  bool success;
+  bool success{false};
+  command_result esp_modem_command_result{command_result::TIMEOUT};
+  mutable std::string cached_c_str;
 
-  // Conversion to bool, allowing you to do things like `if (commandResult) {...}`
   operator bool() const { return success; }
+  const char *c_str() const;
 };
 
 class ModemComponent : public Component {
  public:
-  void set_reboot_timeout(uint16_t timeout) { this->timeout_ = timeout; }
+  void set_reboot_timeout(uint32_t timeout) { this->timeout_ = timeout; }
   void set_use_address(const std::string &use_address) { this->use_address_ = use_address; }
   void set_rx_pin(InternalGPIOPin *rx_pin) { this->rx_pin_ = rx_pin; }
   void set_tx_pin(InternalGPIOPin *tx_pin) { this->tx_pin_ = tx_pin; }
@@ -72,8 +74,8 @@ class ModemComponent : public Component {
   bool is_disabled() { return this->component_state_ == ModemComponentState::DISABLED; }
   bool is_modem_connected(bool verbose);  // this if for modem only, not PPP
   bool is_modem_connected() { return this->is_modem_connected(true); }
+  AtCommandResult send_at(const std::string &cmd) { return this->send_at(cmd, this->command_delay_); }
   AtCommandResult send_at(const std::string &cmd, uint32_t timeout);
-  AtCommandResult send_at(const std::string &cmd);
   AtCommandResult get_imei();
   bool get_power_status();
   bool modem_ready();
@@ -115,9 +117,10 @@ class ModemComponent : public Component {
   void abort_(const std::string &message);
   static void ip_event_handler(void *arg, esp_event_base_t event_base, int32_t event_id, void *event_data);
   void dump_connect_params_();
+  bool flush_uart_();
 
   // Attributes from yaml config
-  uint16_t timeout_;
+  uint32_t timeout_;
   InternalGPIOPin *tx_pin_;
   InternalGPIOPin *rx_pin_;
   std::string model_;