diff --git a/esphome/components/mcp4461/mcp4461.cpp b/esphome/components/mcp4461/mcp4461.cpp index b5ce591547..2b84997b99 100644 --- a/esphome/components/mcp4461/mcp4461.cpp +++ b/esphome/components/mcp4461/mcp4461.cpp @@ -10,9 +10,10 @@ static const char *const TAG = "mcp4461"; constexpr uint8_t EEPROM_WRITE_TIMEOUT_MS = 10; void Mcp4461Component::setup() { - ESP_LOGCONFIG(TAG, "Setting up mcp4461 (0x%02" PRIX8 ")...", this->address_); + ESP_LOGCONFIG(TAG, "Setting up mcp4461 using address (0x%02" PRIX8 ")...", this->address_); auto err = this->write(nullptr, 0); if (err != i2c::ERROR_OK) { + this->error_code_ = MCP4461_STATUS_I2C_ERROR; this->mark_failed(); return; } @@ -22,7 +23,6 @@ void Mcp4461Component::setup() { void Mcp4461Component::begin_() { // save WP/WL status this->set_write_protection_status_(); - this->previous_write_exec_time_ = 0; for (uint8_t i = 0; i < 8; i++) { if (this->reg_[i].initial_value.has_value()) { uint16_t initial_state; @@ -43,6 +43,32 @@ void Mcp4461Component::begin_() { } } +// Converts a status to a human readable string +static const LogString *mcp4461_get_message_string_(int status) { + switch (status) { + case Mcp4461Component::MCP4461_STATUS_I2C_ERROR: + return LOG_STR("I2C error - communication with MCP4461 failed!"); + case Mcp4461Component::MCP4461_STATUS_REGISTER_ERROR: + return LOG_STR("Status register could not be read"); + case Mcp4461Component::MCP4461_STATUS_REGISTER_INVALID: + return LOG_STR("Invalid status register value - bits 1,7 or 8 are 0"); + case Mcp4461Component::MCP4461_VALUE_INVALID: + return LOG_STR("Invalid value for wiper given"); + case Mcp4461Component::MCP4461_WRITE_PROTECTED: + return LOG_STR("MCP4461 is write protected. Setting nonvolatile wipers/eeprom values is prohibited."); + case Mcp4461Component::MCP4461_WIPER_ENABLED: + return LOG_STR("MCP4461 Wiper is already enabled, ignoring cmd to enable."); + case Mcp4461Component::MCP4461_WIPER_DISABLED: + return LOG_STR("MCP4461 Wiper is disabled. All actions on this wiper are prohibited."); + case Mcp4461Component::MCP4461_WIPER_LOCKED: + return LOG_STR("MCP4461 Wiper is locked using WiperLock-technology. All actions on this wiper are prohibited."); + case Mcp4461Component:::MCP4461_STATUS_OK: + return LOG_STR("Status OK"); + default: + return LOG_STR("Unknown"); + } +} + void Mcp4461Component::set_initial_value(Mcp4461WiperIdx wiper, float initial_value) { uint8_t wiper_id = static_cast(wiper); this->reg_[wiper_id].initial_value = initial_value; @@ -61,6 +87,9 @@ void Mcp4461Component::set_write_protection_status_() { void Mcp4461Component::dump_config() { ESP_LOGCONFIG(TAG, "mcp4461:"); LOG_I2C_DEVICE(this); + if (this->is_failed()) { + ESP_LOGE(TAG, "%s", LOG_STR_ARG(mcp4461_get_message_string_(this->error_code_))); + } // log wiper status for (uint8_t i = 0; i < 8; ++i) { // terminals only valid for volatile wipers 0-3 - enable/disable is terminal hw @@ -99,9 +128,6 @@ void Mcp4461Component::dump_config() { ((status_register_value >> 5) & 0x01), ((status_register_value >> 4) & 0x01), ((status_register_value >> 3) & 0x01), ((status_register_value >> 2) & 0x01), ((status_register_value >> 1) & 0x01), ((status_register_value >> 0) & 0x01)); - if (this->is_failed()) { - ESP_LOGE(TAG, "Communication with mcp4461 failed!"); - } } void Mcp4461Component::loop() { @@ -138,16 +164,16 @@ void Mcp4461Component::loop() { uint8_t Mcp4461Component::get_status_register() { if (this->is_failed()) { - ESP_LOGE(TAG, "%s", LOG_STR_ARG(LOG_PARENT_FAILED_STR)); - return 0; + ESP_LOGE(TAG, "%s", LOG_STR_ARG(mcp4461_get_message_string_(this->error_code_))); + return; } uint8_t reg = 0; reg |= static_cast(Mcp4461Addresses::MCP4461_STATUS); reg |= static_cast(Mcp4461Commands::READ); uint16_t buf; if (!this->read_byte_16(reg, &buf)) { + this->error_code_ = MCP4461_STATUS_REGISTER_ERROR; this->mark_failed(); - ESP_LOGE(TAG, "Error fetching status register value"); return 0; } uint8_t msb = buf >> 8; @@ -156,6 +182,7 @@ uint8_t Mcp4461Component::get_status_register() { if (msb != 1 || ((lsb >> 7) & 0x01) != 1 || ((lsb >> 1) & 0x01) != 1) { // D8, D7 and R1 bits are hardlocked to 1 -> a status msb bit 0 (bit 9 of status register) of 0 or lsb bit 1/7 = 0 // indicate device/communication issues, therefore mark component failed + this->error_code_ = MCP4461_STATUS_REGISTER_INVALID; this->mark_failed(); return 0; } @@ -195,9 +222,13 @@ uint8_t Mcp4461Component::get_wiper_address_(uint8_t wiper) { uint16_t Mcp4461Component::get_wiper_level(Mcp4461WiperIdx wiper) { if (this->is_failed()) { - ESP_LOGE(TAG, "%s", LOG_STR_ARG(LOG_PARENT_FAILED_STR)); + ESP_LOGE(TAG, "%s", LOG_STR_ARG(mcp4461_get_message_string_(this->error_code_))); return 0; } + if (!(this->reg_[wiper_idx].enabled)) { + ESP_LOGW(TAG, "%s", LOG_STR_ARG(mcp4461_get_message_string_(MCP4461_WIPER_DISABLED))); + return; + } uint8_t wiper_idx = static_cast(wiper); if (!(this->reg_[wiper_idx].enabled)) { ESP_LOGW(TAG, "reading from disabled volatile wiper %" PRIu8 ", returning 0", wiper_idx); @@ -212,11 +243,12 @@ uint16_t Mcp4461Component::read_wiper_level_(uint8_t wiper) { reg |= this->get_wiper_address_(wiper); reg |= static_cast(Mcp4461Commands::READ); if (wiper > 3) { - if (this->is_eeprom_busy_()) { + if (!this->is_eeprom_ready_for_writing_(true)) { return 0; } } if (!(this->read_byte_16(reg, &buf))) { + this->error_code_ = MCP4461_STATUS_I2C_ERROR; this->status_set_warning(); ESP_LOGW(TAG, "Error fetching %swiper %" PRIu8 " value", (wiper > 3) ? "nonvolatile " : "", wiper); return 0; @@ -225,6 +257,14 @@ uint16_t Mcp4461Component::read_wiper_level_(uint8_t wiper) { } void Mcp4461Component::update_wiper_level(Mcp4461WiperIdx wiper) { + if (this->is_failed()) { + ESP_LOGE(TAG, "%s", LOG_STR_ARG(mcp4461_get_message_string_(this->error_code_))); + return 0; + } + if (!(this->reg_[wiper_idx].enabled)) { + ESP_LOGW(TAG, "%s", LOG_STR_ARG(mcp4461_get_message_string_(MCP4461_WIPER_DISABLED))); + return; + } uint8_t wiper_idx = static_cast(wiper); uint16_t data; data = this->get_wiper_level(wiper); @@ -234,22 +274,22 @@ void Mcp4461Component::update_wiper_level(Mcp4461WiperIdx wiper) { void Mcp4461Component::set_wiper_level(Mcp4461WiperIdx wiper, uint16_t value) { if (this->is_failed()) { - ESP_LOGE(TAG, "%s", LOG_STR_ARG(LOG_PARENT_FAILED_STR)); + ESP_LOGE(TAG, "%s", LOG_STR_ARG(mcp4461_get_message_string_(this->error_code_))); return; } uint8_t wiper_idx = static_cast(wiper); - if (this->reg_[wiper_idx].wiper_lock_active) { - ESP_LOGW(TAG, "Ignoring request to set the state for wiper %" PRIu8 " as it is locked by WiperLock", wiper_idx); - return; - } if (value > 0x100) { - ESP_LOGW(TAG, "ignoring invalid wiper level %" PRIu16 "!"); + ESP_LOGW(TAG, "%s", LOG_STR_ARG(mcp4461_get_message_string_(MCP4461_VALUE_INVALID))); return; } if (!(this->reg_[wiper_idx].enabled)) { - ESP_LOGW(TAG, "writing to disabled volatile wiper %" PRIu8 " is prohibited", wiper_idx); + ESP_LOGW(TAG, "%s", LOG_STR_ARG(mcp4461_get_message_string_(MCP4461_WIPER_DISABLED))); return; } + if (this->reg_[wiper_idx].wiper_lock_active) { + ESP_LOGW(TAG, "%s", LOG_STR_ARG(mcp4461_get_message_string_(MCP4461_WIPER_LOCKED))); + return false; + } ESP_LOGV(TAG, "Setting MCP4461 wiper %" PRIu8 " to %" PRIu16 "!", wiper_idx, value); this->reg_[wiper_idx].state = value; this->update_ = true; @@ -261,17 +301,26 @@ void Mcp4461Component::write_wiper_level_(uint8_t wiper, uint16_t value) { nonvolatile = true; } if (!(this->mcp4461_write_(this->get_wiper_address_(wiper), value, nonvolatile))) { + this->error_code_ = MCP4461_STATUS_I2C_ERROR; + this->status_set_warning(); ESP_LOGW(TAG, "Error writing %swiper %" PRIu8 " level %" PRIu16 "", (wiper > 3) ? "nonvolatile " : "", wiper, value); - this->status_set_warning(); } } void Mcp4461Component::enable_wiper(Mcp4461WiperIdx wiper) { if (this->is_failed()) { - ESP_LOGE(TAG, "%s", LOG_STR_ARG(LOG_PARENT_FAILED_STR)); + ESP_LOGE(TAG, "%s", LOG_STR_ARG(mcp4461_get_message_string_(this->error_code_))); return; } + if ((this->reg_[wiper_idx].enabled)) { + ESP_LOGW(TAG, "%s", LOG_STR_ARG(mcp4461_get_message_string_(MCP4461_WIPER_ENABLED))); + return; + } + if (this->reg_[wiper_idx].wiper_lock_active) { + ESP_LOGW(TAG, "%s", LOG_STR_ARG(mcp4461_get_message_string_(MCP4461_WIPER_LOCKED))); + return false; + } uint8_t wiper_idx = static_cast(wiper); ESP_LOGV(TAG, "Enabling wiper %" PRIu8, wiper_idx); this->reg_[wiper_idx].terminal_hw = true; @@ -280,9 +329,17 @@ void Mcp4461Component::enable_wiper(Mcp4461WiperIdx wiper) { void Mcp4461Component::disable_wiper(Mcp4461WiperIdx wiper) { if (this->is_failed()) { - ESP_LOGE(TAG, "%s", LOG_STR_ARG(LOG_PARENT_FAILED_STR)); + ESP_LOGE(TAG, "%s", LOG_STR_ARG(mcp4461_get_message_string_(this->error_code_))); return; } + if (!(this->reg_[wiper_idx].enabled)) { + ESP_LOGW(TAG, "%s", LOG_STR_ARG(mcp4461_get_message_string_(MCP4461_WIPER_DISABLED))); + return; + } + if (this->reg_[wiper_idx].wiper_lock_active) { + ESP_LOGW(TAG, "%s", LOG_STR_ARG(mcp4461_get_message_string_(MCP4461_WIPER_LOCKED))); + return false; + } uint8_t wiper_idx = static_cast(wiper); ESP_LOGV(TAG, "Disabling wiper %" PRIu8, wiper_idx); this->reg_[wiper_idx].terminal_hw = false; @@ -291,16 +348,16 @@ void Mcp4461Component::disable_wiper(Mcp4461WiperIdx wiper) { bool Mcp4461Component::increase_wiper(Mcp4461WiperIdx wiper) { if (this->is_failed()) { - ESP_LOGE(TAG, "%s", LOG_STR_ARG(LOG_PARENT_FAILED_STR)); + ESP_LOGE(TAG, "%s", LOG_STR_ARG(mcp4461_get_message_string_(this->error_code_))); return false; } uint8_t wiper_idx = static_cast(wiper); if (!(this->reg_[wiper_idx].enabled)) { - ESP_LOGW(TAG, "increasing disabled volatile wiper %" PRIu8 " is prohibited", wiper_idx); - return false; + ESP_LOGW(TAG, "%s", LOG_STR_ARG(mcp4461_get_message_string_(MCP4461_WIPER_DISABLED))); + return; } if (this->reg_[wiper_idx].wiper_lock_active) { - ESP_LOGW(TAG, "Ignoring request to increase wiper %" PRIu8 " as it is locked by WiperLock", wiper_idx); + ESP_LOGW(TAG, "%s", LOG_STR_ARG(mcp4461_get_message_string_(MCP4461_WIPER_LOCKED))); return false; } ESP_LOGV(TAG, "Increasing wiper %" PRIu8 "", wiper_idx); @@ -311,6 +368,7 @@ bool Mcp4461Component::increase_wiper(Mcp4461WiperIdx wiper) { reg |= static_cast(Mcp4461Commands::INCREMENT); auto err = this->write(&this->address_, reg, sizeof(reg)); if (err != i2c::ERROR_OK) { + this->error_code_ = MCP4461_STATUS_I2C_ERROR; this->status_set_warning(); return false; } @@ -320,16 +378,16 @@ bool Mcp4461Component::increase_wiper(Mcp4461WiperIdx wiper) { bool Mcp4461Component::decrease_wiper(Mcp4461WiperIdx wiper) { if (this->is_failed()) { - ESP_LOGE(TAG, "%s", LOG_STR_ARG(LOG_PARENT_FAILED_STR)); + ESP_LOGE(TAG, "%s", LOG_STR_ARG(mcp4461_get_message_string_(this->error_code_))); return false; } uint8_t wiper_idx = static_cast(wiper); if (!(this->reg_[wiper_idx].enabled)) { - ESP_LOGW(TAG, "decreasing disabled volatile wiper %" PRIu8 " is prohibited", wiper_idx); - return false; + ESP_LOGW(TAG, "%s", LOG_STR_ARG(mcp4461_get_message_string_(MCP4461_WIPER_DISABLED))); + return; } if (this->reg_[wiper_idx].wiper_lock_active) { - ESP_LOGW(TAG, "Ignoring request to decrease wiper %" PRIu8 " as it is locked by WiperLock", wiper_idx); + ESP_LOGW(TAG, "%s", LOG_STR_ARG(mcp4461_get_message_string_(MCP4461_WIPER_LOCKED))); return false; } ESP_LOGV(TAG, "Decreasing wiper %" PRIu8 "", wiper_idx); @@ -340,6 +398,7 @@ bool Mcp4461Component::decrease_wiper(Mcp4461WiperIdx wiper) { reg |= static_cast(Mcp4461Commands::DECREMENT); auto err = this->write(&this->address_, reg, sizeof(reg)); if (err != i2c::ERROR_OK) { + this->error_code_ = MCP4461_STATUS_I2C_ERROR; this->status_set_warning(); return false; } @@ -373,7 +432,7 @@ uint8_t Mcp4461Component::calc_terminal_connector_byte_(Mcp4461TerminalIdx termi uint8_t Mcp4461Component::get_terminal_register(Mcp4461TerminalIdx terminal_connector) { if (this->is_failed()) { - ESP_LOGE(TAG, "%s", LOG_STR_ARG(LOG_PARENT_FAILED_STR)); + ESP_LOGE(TAG, "%s", LOG_STR_ARG(mcp4461_get_message_string_(this->error_code_))); return 0; } uint8_t reg = 0; @@ -387,6 +446,7 @@ uint8_t Mcp4461Component::get_terminal_register(Mcp4461TerminalIdx terminal_conn if (this->read_byte_16(reg, &buf)) { return static_cast(buf & 0x00ff); } else { + this->error_code_ = MCP4461_STATUS_I2C_ERROR; this->status_set_warning(); ESP_LOGW(TAG, "Error fetching terminal register value"); return 0; @@ -423,7 +483,7 @@ void Mcp4461Component::update_terminal_register(Mcp4461TerminalIdx terminal_conn bool Mcp4461Component::set_terminal_register(Mcp4461TerminalIdx terminal_connector, uint8_t data) { if (this->is_failed()) { - ESP_LOGE(TAG, "%s", LOG_STR_ARG(LOG_PARENT_FAILED_STR)); + ESP_LOGE(TAG, "%s", LOG_STR_ARG(mcp4461_get_message_string_(this->error_code_))); return false; } uint8_t addr; @@ -436,6 +496,7 @@ bool Mcp4461Component::set_terminal_register(Mcp4461TerminalIdx terminal_connect return false; } if (!(this->mcp4461_write_(addr, data))) { + this->error_code_ = MCP4461_STATUS_I2C_ERROR; this->status_set_warning(); return false; } @@ -444,7 +505,7 @@ bool Mcp4461Component::set_terminal_register(Mcp4461TerminalIdx terminal_connect void Mcp4461Component::enable_terminal(Mcp4461WiperIdx wiper, char terminal) { if (this->is_failed()) { - ESP_LOGE(TAG, "%s", LOG_STR_ARG(LOG_PARENT_FAILED_STR)); + ESP_LOGE(TAG, "%s", LOG_STR_ARG(mcp4461_get_message_string_(this->error_code_))); return; } uint8_t wiper_idx = static_cast(wiper); @@ -474,7 +535,7 @@ void Mcp4461Component::enable_terminal(Mcp4461WiperIdx wiper, char terminal) { void Mcp4461Component::disable_terminal(Mcp4461WiperIdx wiper, char terminal) { if (this->is_failed()) { - ESP_LOGE(TAG, "%s", LOG_STR_ARG(LOG_PARENT_FAILED_STR)); + ESP_LOGE(TAG, "%s", LOG_STR_ARG(mcp4461_get_message_string_(this->error_code_))); return; } uint8_t wiper_idx = static_cast(wiper); @@ -504,17 +565,18 @@ void Mcp4461Component::disable_terminal(Mcp4461WiperIdx wiper, char terminal) { uint16_t Mcp4461Component::get_eeprom_value(Mcp4461EepromLocation location) { if (this->is_failed()) { - ESP_LOGE(TAG, "%s", LOG_STR_ARG(LOG_PARENT_FAILED_STR)); + ESP_LOGE(TAG, "%s", LOG_STR_ARG(mcp4461_get_message_string_(this->error_code_))); return 0; } uint8_t reg = 0; reg |= static_cast(Mcp4461EepromLocation::MCP4461_EEPROM_1) + (static_cast(location) * 0x10); reg |= static_cast(Mcp4461Commands::READ); uint16_t buf; - if (this->is_eeprom_busy_()) { + if (!this->is_eeprom_ready_for_writing_(true)) { return 0; } if (!this->read_byte_16(reg, &buf)) { + this->error_code_ = MCP4461_STATUS_I2C_ERROR; this->status_set_warning(); ESP_LOGW(TAG, "Error fetching EEPRom location value"); return 0; @@ -524,7 +586,7 @@ uint16_t Mcp4461Component::get_eeprom_value(Mcp4461EepromLocation location) { bool Mcp4461Component::set_eeprom_value(Mcp4461EepromLocation location, uint16_t value) { if (this->is_failed()) { - ESP_LOGE(TAG, "%s", LOG_STR_ARG(LOG_PARENT_FAILED_STR)); + ESP_LOGE(TAG, "%s", LOG_STR_ARG(mcp4461_get_message_string_(this->error_code_))); return false; } uint8_t addr = 0; @@ -536,26 +598,96 @@ bool Mcp4461Component::set_eeprom_value(Mcp4461EepromLocation location, uint16_t } addr |= static_cast(Mcp4461EepromLocation::MCP4461_EEPROM_1) + (static_cast(location) * 0x10); if (!(this->mcp4461_write_(addr, value, true))) { + this->error_code_ = MCP4461_STATUS_I2C_ERROR; this->status_set_warning(); + ESP_LOGW(TAG, "Error writing EEPRom value"); return false; } return true; } -bool Mcp4461Component::is_writing_() { return static_cast((this->get_status_register() >> 4) & 0x01); } +/** + * @brief Checks if the EEPROM is currently writing. + * + * This function reads the MCP4461 status register to determine if an EEPROM write operation is in progress. + * If the EEPROM is no longer writing, the timeout flag (`last_eeprom_write_timed_out_`) + * is reset to allow normal operation in future calls of `is_eeprom_ready_for_writing_()`. + * + * Behavior: + * - If the EEPROM is **writing**, the function returns `true`. + * - If the EEPROM is **not writing**, the function returns `false` and resets `last_eeprom_write_timed_out_`. + * + * @return `true` if the EEPROM is currently writing. + * @return `false` if the EEPROM is not writing (also resets the timeout flag). + */ +bool Mcp4461Component::is_writing_() { + /* Read the EEPROM write-active status from the status register */ + bool writing = static_cast((this->get_status_register() >> 4) & 0x01); -bool Mcp4461Component::is_eeprom_busy_() { - while (this->is_writing_() && this->previous_write_exec_time_ != 0) { - if ((millis() - this->previous_write_exec_time_) > EEPROM_WRITE_TIMEOUT_MS) { - this->previous_write_exec_time_ = millis(); - ESP_LOGE(TAG, - "EEPROM write timeout exceeded (%" PRIu8 " ms), aborting read/write from/to nonvolatile wiper/eeprom", - EEPROM_WRITE_TIMEOUT_MS); + /* If EEPROM is no longer writing, reset the timeout flag */ + if (!writing) { + /* This is protected boolean flag in Mcp4461Component class */ + this->last_eeprom_write_timed_out_ = false; + } + + return writing; +} + +/** + * @brief Checks if the EEPROM is ready for a new write operation. + * + * This function ensures that the EEPROM is not actively writing. + * It can either return the current status immediately or wait until the EEPROM becomes ready. + * + * Behavior: + * - If `wait_if_not_ready` is `false`, the function returns the current readiness status immediately. + * - If `wait_if_not_ready` is `true`: + * - The function waits up to `EEPROM_WRITE_TIMEOUT_MS` for the EEPROM to become ready. + * - If the EEPROM remains busy after the timeout, the `last_eeprom_write_timed_out_` flag is set to `true`, + * preventing unnecessary waits in future calls. + * - If the EEPROM becomes ready within the timeout, the function returns `true`. + * + * @param[in] wait_if_not_ready Specifies whether to wait for EEPROM readiness if it is currently busy. + * - `true` → Waits for completion (up to `EEPROM_WRITE_TIMEOUT_MS`). + * - `false` → Returns the current readiness status without waiting. + * + * @return `true` if the EEPROM is ready for a new write. + * @return `false` if: + * - The last write attempt **timed out** (`wait_if_not_ready = true`). + * - The EEPROM is still busy (`wait_if_not_ready = false`). + */ +bool Mcp4461Component::is_eeprom_ready_for_writing_(bool wait_if_not_ready) { + /* Check initial write status */ + bool ready_for_write = !this->is_writing_(); + + /* Return early if no waiting is required or EEPROM is already ready */ + if (ready_for_write || !wait_if_not_ready || this->last_eeprom_write_timed_out_) { + return ready_for_write; + } + + /* Timestamp before starting the loop */ + const uint32_t start_millis = millis(); + + ESP_LOGV(TAG, "Waiting until EEPROM is ready for write, start_millis = %" PRIu32, start_millis); + + /* Loop until EEPROM is ready or timeout is reached */ + while (!ready_for_write && ((millis() - start_millis) < EEPROM_WRITE_TIMEOUT_MS)) { + ready_for_write = !this->is_writing_(); + + /* If ready, exit early */ + if (ready_for_write) { + ESP_LOGV(TAG, "EEPROM is ready for new write, elapsed_millis = %" PRIu32, millis() - start_millis); return true; } - ESP_LOGV(TAG, "Waiting while eeprom is busy"); + + /* Not ready yet, yield before checking again */ yield(); } + + /* If still not ready after timeout, log error and mark the timeout */ + ESP_LOGE(TAG, "EEPROM write timeout exceeded (%" PRIu8 " ms)", EEPROM_WRITE_TIMEOUT_MS); + this->last_eeprom_write_timed_out_ = true; + return false; } @@ -571,13 +703,12 @@ bool Mcp4461Component::mcp4461_write_(uint8_t addr, uint16_t data, bool nonvolat reg |= static_cast(Mcp4461Commands::WRITE); if (nonvolatile) { if (this->write_protected_) { - ESP_LOGW(TAG, "Ignoring write to write protected chip"); + ESP_LOGW(TAG, "%s", LOG_STR_ARG(mcp4461_get_message_string_(MCP4461_WRITE_PROTECTED))); return false; } - if (this->is_eeprom_busy_()) { + if (!this->is_eeprom_ready_for_writing_(true)) { return false; } - this->previous_write_exec_time_ = millis(); } return this->write_byte(reg, value_byte); } diff --git a/esphome/components/mcp4461/mcp4461.h b/esphome/components/mcp4461/mcp4461.h index 70b0c5561a..cb9ff0cfaa 100644 --- a/esphome/components/mcp4461/mcp4461.h +++ b/esphome/components/mcp4461/mcp4461.h @@ -7,8 +7,6 @@ namespace esphome { namespace mcp4461 { -static const LogString *const LOG_PARENT_FAILED_STR = LOG_STR("Parent MCP4461 component has failed! Aborting"); - struct WiperState { bool terminal_a = true; bool terminal_b = true; @@ -55,6 +53,16 @@ enum class Mcp4461EepromLocation : uint8_t { enum class Mcp4461TerminalIdx : uint8_t { MCP4461_TERMINAL_0 = 0, MCP4461_TERMINAL_1 = 1 }; +enum ErrorCode { + MCP4461_STATUS_OK = 0, // CMD completed successfully + MCP4461_STATUS_I2C_ERROR, // Unable to communicate with device + MCP4461_STATUS_REGISTER_INVALID, // Status register value was invalid + MCP4461_VALUE_INVALID, // Invalid value given for wiper / eeprom + MCP4461_STATUS_WRITE_PROTECTED, // The value was read, but the CRC over the payload (valid and data) does not match + MCP4461_STATUS_WIPER_LOCKED, // The wiper is locked using WiperLock-technology - all actions for this wiper will be aborted/discarded +} error_code_{MCP4461_STATUS_OK}; + + class Mcp4461Wiper; // Mcp4461Component @@ -96,10 +104,10 @@ class Mcp4461Component : public Component, public i2c::I2CDevice { protected: friend class Mcp4461Wiper; void set_write_protection_status_(); - bool is_writing_(); - bool is_eeprom_busy_(); uint8_t get_wiper_address_(uint8_t wiper); uint16_t read_wiper_level_(uint8_t wiper); + bool is_writing_(); + bool is_eeprom_ready_for_writing_(bool wait_if_not_ready); void write_wiper_level_(uint8_t wiper, uint16_t value); bool mcp4461_write_(uint8_t addr, uint16_t data, bool nonvolatile = false); uint8_t calc_terminal_connector_byte_(Mcp4461TerminalIdx terminal_connector); @@ -107,7 +115,7 @@ class Mcp4461Component : public Component, public i2c::I2CDevice { WiperState reg_[8]; void begin_(); bool update_{false}; - uint32_t previous_write_exec_time_; + bool last_eeprom_write_timed_out_{false}; bool write_protected_{false}; bool wiper_0_disabled_{false}; bool wiper_1_disabled_{false};