diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 735ba73c99..7e695bb46b 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -466,7 +466,7 @@ jobs: echo "binary=$BINARY" >> $GITHUB_OUTPUT - name: Run CodSpeed benchmarks - uses: CodSpeedHQ/action@0ca9cbbf4623b599a6c3ed4fc8a922942705d9f1 # v5.0.2 + uses: CodSpeedHQ/action@4296e51e7041e24dadb86d1d6e8b9320d223dbe8 # v5.0.3 with: run: | . venv/bin/activate diff --git a/AGENTS.md b/AGENTS.md index 40381030cb..fa0f61c263 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -57,6 +57,12 @@ This document provides essential context for AI models interacting with this pro - Function-local constants: `lower_snake_case` - Protected/private fields: `lower_snake_case_with_trailing_underscore_` - Favor descriptive names over abbreviations + - Enumerator names: prefix every value of an `enum class` with the enum name converted to + `UPPER_SNAKE_CASE` (e.g. `UARTFlushResult::UART_FLUSH_RESULT_SUCCESS`). Never use bare + names like `SUCCESS`, `FAILURE`, `OK`, or `FAIL`: platform SDK headers define macros with + these common names (for example the Realtek SDKs used by LibreTiny define + `#define SUCCESS 0` in `basic_types.h`), and the preprocessor replaces the enumerator + before the compiler sees it, breaking the build and clang-tidy on those platforms. * **Python Idioms:** * **Assignment expressions (PEP 572):** Prefer the walrus operator (`:=`) wherever it removes a redundant lookup or a throwaway temporary. The most common case in component code is presence-checking a config key and then indexing it separately — fetch once with `.get()` and bind in the condition instead: diff --git a/esphome/components/bk72xx_ble/bdk_scan.cpp b/esphome/components/bk72xx_ble/bdk_scan.cpp new file mode 100644 index 0000000000..bd4e51d9b7 --- /dev/null +++ b/esphome/components/bk72xx_ble/bdk_scan.cpp @@ -0,0 +1,119 @@ +// Every SDK call the scan reconciler makes. The BDK's own start hardcodes +// passive (the active bit is commented out in both stacks), so +// bdk_scan_start() packs the GAPM_ACTIVITY_START_CMD itself, field-for-field +// the SDK's app_ble_start_scaning() except that prop takes the mode, armed +// through the SDK's own operation bookkeeping. The component pins +// beken-bdk 3.0.78; the static asserts catch a layout change on a bump. + +#include "bdk_scan.h" + +#ifdef USE_BK72XX_BLE + +// Same SDK gate as bk72xx_ble.cpp (which carries the explanatory #error). +#if !defined(CLANG_TIDY) && __has_include("ble_api.h") + +extern "C" { +#include "app_ble.h" // app_ble_env, app_ble_run, app_ble_reset, actv_state_t, + // app_ble_actv_state_get, app_ble_env_state_get, + // app_ble_get_idle_actv_idx_handle, UNKNOW_ACT_IDX, + // bk_ble_* (via ble_api_5_x.h) +#include "kernel_msg.h" // KERNEL_MSG_ALLOC, kernel_msg_send +#if __has_include("gapm_msg.h") +#include "gapm_msg.h" // BLE 5.2 (BK7238/BK7252N): gapm_activity_start_cmd, GAPM_SCAN_* +#else +#include "gapm_task.h" // BLE 5.1 (BK7231N/BK7236): same declarations, older header name +#endif +} + +#include "esphome/core/log.h" + +namespace esphome::bk72xx_ble { + +static const char *const TAG = "bk72xx_ble"; + +// Pin the SDK surface this file depends on: a beken-bdk bump that moves these +// must fail the build, not corrupt the kernel message. +static_assert(GAPM_SCAN_PROP_PHY_1M_BIT == (1 << 0) && GAPM_SCAN_PROP_ACTIVE_1M_BIT == (1 << 2) && + sizeof(struct gapm_scan_param) == 16 && sizeof(struct gapm_scan_wd_op_param) == 4, + "beken-bdk GAPM scan layout changed; revalidate bdk_scan_start() " + "against the SDK's app_ble_start_scaning()"); +static_assert(INVALID_ACTIVITY_IDX == UNKNOW_ACT_IDX, + "beken-bdk activity sentinel changed; revalidate the scan reconciler"); +static_assert(GAPM_REPORT_TYPE_SCAN_RSP_EXT == 2 && GAPM_REPORT_TYPE_SCAN_RSP_LEG == 3 && + GAPM_REPORT_INFO_SCAN_ADV_BIT == (1 << 5), + "beken-bdk GAPM report info changed; revalidate the tracker's demux constants"); + +bool bdk_scan_ready() { return app_ble_env_state_get() == APP_BLE_READY; } + +BdkActivityState bdk_scan_state(uint8_t activity_idx) { + if (activity_idx == INVALID_ACTIVITY_IDX) + return BdkActivityState::IDLE; + switch (app_ble_actv_state_get(activity_idx)) { + case ACTV_IDLE: + return BdkActivityState::IDLE; + case ACTV_SCAN_CREATED: + return BdkActivityState::CREATED; + case ACTV_SCAN_STARTED: + return BdkActivityState::STARTED; + default: + return BdkActivityState::OTHER; + } +} + +uint8_t bdk_scan_acquire_activity() { + uint8_t idx = app_ble_get_idle_actv_idx_handle(SCAN_ACTV); + if (idx == INVALID_ACTIVITY_IDX) + ESP_LOGE(TAG, "Scan start failed: no idle activity handle"); + return idx; +} + +BdkOpResult bdk_scan_create(uint8_t activity_idx) { + ble_err_t ret = bk_ble_create_scaning(activity_idx, nullptr); + if (ret == ERR_SUCCESS) + return BdkOpResult::OK; + if (ret == ERR_BLE_STATUS) + return BdkOpResult::BUSY; + ESP_LOGE(TAG, "Scan activity create failed (err %d)", static_cast(ret)); + return BdkOpResult::FAILED; +} + +BdkOpResult bdk_scan_start(uint8_t activity_idx, uint16_t interval, uint16_t window, bool active) { + app_ble_run(activity_idx, BLE_START_SCAN, 1 << BLE_OP_START_SCAN_POS, nullptr); + struct gapm_activity_start_cmd *cmd = + KERNEL_MSG_ALLOC(GAPM_ACTIVITY_START_CMD, TASK_BLE_GAPM, TASK_BLE_APP, gapm_activity_start_cmd); + if (cmd == nullptr) { + app_ble_reset(); // the SDK's own failure path for an unsent operation + ESP_LOGE(TAG, "Scan start failed: kernel message allocation"); + return BdkOpResult::FAILED; + } + cmd->operation = GAPM_START_ACTIVITY; + cmd->actv_idx = app_ble_env.actvs[activity_idx].gap_advt_idx; + cmd->u_param.scan_param.type = GAPM_SCAN_TYPE_OBSERVER; + cmd->u_param.scan_param.prop = GAPM_SCAN_PROP_PHY_1M_BIT | (active ? GAPM_SCAN_PROP_ACTIVE_1M_BIT : 0); + cmd->u_param.scan_param.scan_param_1m.scan_intv = interval; + cmd->u_param.scan_param.scan_param_1m.scan_wd = window; + cmd->u_param.scan_param.scan_param_coded.scan_intv = 0; + cmd->u_param.scan_param.scan_param_coded.scan_wd = 0; + cmd->u_param.scan_param.dup_filt_pol = 0; + cmd->u_param.scan_param.rsvd = 0; + cmd->u_param.scan_param.duration = 0; // scan until stopped + cmd->u_param.scan_param.period = 10; // matches the SDK's passive start + kernel_msg_send(cmd); + return BdkOpResult::OK; +} + +BdkOpResult bdk_scan_release(uint8_t activity_idx, bool created, int *err_out) { + ble_err_t ret = created ? bk_ble_delete_scaning(activity_idx, nullptr) : bk_ble_scan_stop(activity_idx, nullptr); + *err_out = static_cast(ret); + if (ret == ERR_SUCCESS) + return BdkOpResult::OK; + // DEBUG on purpose: the reconciler WARNs once per streak and the stuck + // ERROR carries this code — a per-retry ERROR would be unbounded. + ESP_LOGD(TAG, "Scan release %s (err %d)", ret == ERR_BLE_STATUS ? "rejected" : "failed", static_cast(ret)); + return ret == ERR_BLE_STATUS ? BdkOpResult::BUSY : BdkOpResult::FAILED; +} + +} // namespace esphome::bk72xx_ble + +#endif // !CLANG_TIDY && ble_api.h +#endif // USE_BK72XX_BLE diff --git a/esphome/components/bk72xx_ble/bdk_scan.h b/esphome/components/bk72xx_ble/bdk_scan.h new file mode 100644 index 0000000000..47bce2449d --- /dev/null +++ b/esphome/components/bk72xx_ble/bdk_scan.h @@ -0,0 +1,51 @@ +#pragma once + +#include "esphome/core/defines.h" + +#ifdef USE_BK72XX_BLE + +#include + +namespace esphome::bk72xx_ble { + +/// Activity index value marking "no scan activity", the BDK's own convention +/// (asserted against its symbol in bdk_scan.cpp). +inline constexpr uint8_t INVALID_ACTIVITY_IDX = 0xFF; + +/// Scan-relevant controller activity states, read live from the SDK. +enum class BdkActivityState : uint8_t { + IDLE, ///< No activity (or one whose create failed). + CREATED, ///< Created but not started. + STARTED, ///< Scanning. + OTHER, ///< A non-scan or transitional state; settles on a later read. +}; + +/// Outcome of a BDK scan operation request. +enum class BdkOpResult : uint8_t { + OK, ///< Accepted; completion is asynchronous. + BUSY, ///< Another controller operation is in flight; retry later. + FAILED, ///< Rejected. +}; + +/// True when no controller operation is in flight (APP_BLE_READY). +bool bdk_scan_ready(); +/// Live state of the given activity; INVALID_ACTIVITY_IDX reads as IDLE. +BdkActivityState bdk_scan_state(uint8_t activity_idx); +/// Claim an idle activity slot; INVALID_ACTIVITY_IDX when none is free. +uint8_t bdk_scan_acquire_activity(); +/// Create the scan activity (asynchronous); started once CREATED is observed. +BdkOpResult bdk_scan_create(uint8_t activity_idx); +/// Start a created activity: the packed GAPM start, taking the scan mode the +/// BDK's own start path hardcodes away. Fire-and-forget; FAILED when the +/// kernel message could not be allocated (the armed SDK operation is rolled +/// back). +BdkOpResult bdk_scan_start(uint8_t activity_idx, uint16_t interval, uint16_t window, bool active); +/// Release the activity: delete when never started (a stop would be +/// rejected), stop otherwise. BUSY on a transient rejection (retry), FAILED +/// on any other error; err_out receives the SDK code (0 on success). +/// Teardown is asynchronous — observe IDLE to confirm. +BdkOpResult bdk_scan_release(uint8_t activity_idx, bool created, int *err_out); + +} // namespace esphome::bk72xx_ble + +#endif // USE_BK72XX_BLE diff --git a/esphome/components/bk72xx_ble/bk72xx_ble.cpp b/esphome/components/bk72xx_ble/bk72xx_ble.cpp index 69c7df0b96..954cb9fe87 100644 --- a/esphome/components/bk72xx_ble/bk72xx_ble.cpp +++ b/esphome/components/bk72xx_ble/bk72xx_ble.cpp @@ -5,7 +5,8 @@ // talks to the Beken BDK BLE stack: // - one-time stack bring-up (ble_set_notice_cb() + ble_entry()), // - the controller BLE address, -// - the raw controller scan primitives (bk_ble_scan_start/stop), +// - the scan reconciler (request, pacing, bring-up budget) over the +// bdk_scan surface, // - the scan-report ring: the BDK notice callback (BLE task) takes a report // from a fixed pool and pushes it on a lock-free SPSC queue; loop() drains, // dispatches on the main task and returns reports to the pool — the same @@ -20,10 +21,13 @@ #include "bk72xx_ble.h" // pulls esphome/core/defines.h for USE_BK72XX_BLE +#include "bdk_scan.h" // the raw BDK scan surface (state reads, starts, release) + #ifdef USE_BK72XX_BLE #include +#include "esphome/core/application.h" #include "esphome/core/hal.h" #include "esphome/core/helpers.h" // get_mac_address_raw() #include "esphome/core/log.h" @@ -57,9 +61,8 @@ // are C headers consumed from C++ (a standard C-header-from-C++ pattern). // --------------------------------------------------------------------------- extern "C" { -#include "ble_api.h" // bk_ble_scan_start/stop, ble_entry, ble_set_notice_cb, - // app_ble_get_idle_actv_idx_handle, struct scan_param, - // recv_adv_t, ble_notice_t, BLE_5_REPORT_ADV, SCAN_ACTV +#include "ble_api.h" // ble_set_notice_cb, recv_adv_t, ble_notice_t, + // BLE_5_REPORT_ADV (scan primitives live in bdk_scan.cpp) #ifdef BK72XX_BLE_HAS_COMMON_BDADDR #include "common_bt_defines.h" // struct bd_addr // The controller's public BLE address, populated by the BDK during ble_entry(). @@ -76,6 +79,12 @@ namespace esphome::bk72xx_ble { static const char *const TAG = "bk72xx_ble"; +static constexpr uint32_t RECONCILE_RETRY_MS = 10; // pump floor for fast loops +static constexpr uint32_t RECONCILE_REJECTED_RETRY_MS = 500; // retry gate after a rejected release +static constexpr uint32_t RECONCILE_PENDING_TIMEOUT_MS = 2000; // bring-up budget before FAILED +static constexpr uint32_t SCAN_LIVENESS_CHECK_MS = 1000; // settled-scan re-check cadence +static constexpr uint32_t TEARDOWN_STUCK_ERROR_MS = 30000; // stuck-teardown ERROR (stop also goes FAILED) + // The BDK notice callback is a plain C function pointer with no user argument, // so it reaches the (single) component instance through a file-static pointer. static BK72xxBLE *s_ble = nullptr; // NOLINT(cppcoreguidelines-avoid-non-const-global-variables) @@ -95,12 +104,12 @@ static void ble_notice_callback(ble_notice_t notice, void *param) { const recv_adv_t *info = reinterpret_cast(param); // rssi is a signed dBm carried in a uint8_t; cast through int8_t (standard for // a signed dBm value packed in a uint8_t). - s_ble->enqueue_scan_report(info->adv_addr, static_cast(info->rssi), info->adv_addr_type, info->data, - info->data_len); + s_ble->enqueue_scan_report(info->adv_addr, static_cast(info->rssi), info->adv_addr_type, + static_cast(info->evt_type), info->data, info->data_len); } -void BK72xxBLE::enqueue_scan_report(const uint8_t *mac, int8_t rssi, uint8_t addr_type, const uint8_t *data, - uint16_t data_len) { +void BK72xxBLE::enqueue_scan_report(const uint8_t *mac, int8_t rssi, uint8_t addr_type, uint8_t evt_type, + const uint8_t *data, uint16_t data_len) { BLEScanReport *report = this->report_pool_.allocate(); if (report == nullptr) { // Pool exhausted — the queue is full; count and drop. @@ -110,6 +119,7 @@ void BK72xxBLE::enqueue_scan_report(const uint8_t *mac, int8_t rssi, uint8_t add memcpy(report->mac, mac, 6); report->rssi = rssi; report->addr_type = addr_type; + report->evt_type = evt_type; report->data_len = (data_len <= sizeof(report->data)) ? static_cast(data_len) : static_cast(sizeof(report->data)); memcpy(report->data, data, report->data_len); @@ -123,6 +133,9 @@ void BK72xxBLE::enqueue_scan_report(const uint8_t *mac, int8_t rssi, uint8_t add void BK72xxBLE::setup() { s_ble = this; + // The report pool grows lazily on purpose: the BDK notice callback runs in + // task context (malloc-safe, unlike rp2040's IRQ path), and typical traffic + // stays far below the pool cap, so not warming contains RAM. // Resolve the MAC early so get_mac_lsb_first() is valid for consumers before // the stack is up (it is re-read once ble_entry() has run). this->resolve_mac_(); @@ -173,6 +186,30 @@ void BK72xxBLE::enable() { } void BK72xxBLE::loop() { + // Keep reconciling toward the requested scan state (e.g. complete a stop + // that arrived while a controller operation was in flight), and re-check a + // settled scan at low frequency: a controller-side drop re-enters the + // bring-up, and the budget's FAILED feeds the tracker's recovery. + // Keep driving until settled: any PENDING, plus a terminal stop whose slot + // must still be freed. A FAILED scan request is the one combination not + // re-driven here — that belongs to the tracker's backoff. + const uint32_t pump_now = App.get_loop_component_start_time(); + if (this->last_result_ == ScanOpResult::PENDING || + (!this->scan_wanted_ && this->last_result_ == ScanOpResult::FAILED)) { + const uint32_t gate = (this->release_warned_ || this->last_result_ == ScanOpResult::FAILED) + ? RECONCILE_REJECTED_RETRY_MS + : RECONCILE_RETRY_MS; + if (pump_now - this->last_advance_ms_ >= gate) + this->advance_(); + } else if (this->scan_wanted_ && this->last_result_ == ScanOpResult::SETTLED && + pump_now - this->last_advance_ms_ >= SCAN_LIVENESS_CHECK_MS) { + // Re-check a settled scan; scan_start() refills the bring-up budget. + // WARN: the only report of a drop that recovers inside its budget. + if (this->scan_start(this->requested_.interval, this->requested_.window, this->requested_.active) != + ScanOpResult::SETTLED) + ESP_LOGW(TAG, "Controller dropped the scan; restarting"); + } + // Drain the lock-free ring filled by the BLE task; all per-report work runs // here on the main task, then the report returns to the pool. BLEScanReport *report = this->report_queue_.pop(); @@ -248,44 +285,228 @@ void BK72xxBLE::resolve_mac_() { } // --------------------------------------------------------------------------- -// Controller scan primitives +// Scan reconciler // --------------------------------------------------------------------------- -bool BK72xxBLE::scan_start(uint16_t interval, uint16_t window) { +// Episode boundary: fresh teardown deadline and error bookkeeping. +void BK72xxBLE::reset_teardown_episode_() { + this->teardown_since_ms_ = 0; + this->restarting_ = false; + this->last_release_err_ = 0; +} + +ScanOpResult BK72xxBLE::scan_start(uint16_t interval, uint16_t window, bool active) { if (!this->is_active()) this->enable(); - if (this->scan_actv_idx_ != 0xFF) { - // Already scanning — stop first so this call cleanly restarts with the new - // parameters (the BDK cannot start a second scan on a busy activity). - this->scan_stop(); + const ScanParams params{active, interval, window}; + // A new episode refills the budget and gets a fresh teardown deadline; a + // re-call observing an in-flight bring-up (last result PENDING) must not. + if (this->last_result_ != ScanOpResult::PENDING || !this->scan_wanted_ || params != this->requested_) { + this->pending_since_ms_ = App.get_loop_component_start_time(); + this->reset_teardown_episode_(); } + this->scan_wanted_ = true; + this->requested_ = params; + return this->advance_(); +} - struct scan_param sp; - memset(&sp, 0, sizeof(sp)); - sp.channel_map = 7; // advertising channels 37/38/39 - sp.interval = interval; - sp.window = window; +void BK72xxBLE::scan_stop() { + if (this->scan_wanted_) { + // A stamp inherited from a stuck restart would fail the stop on its + // first advance. + this->reset_teardown_episode_(); + } + this->scan_wanted_ = false; + this->advance_(); +} - this->scan_actv_idx_ = app_ble_get_idle_actv_idx_handle(SCAN_ACTV); - if (this->scan_actv_idx_ == 0xFF) { - ESP_LOGE(TAG, "Scan start failed: no idle activity handle"); +bool BK72xxBLE::flush_pending_stop(uint32_t timeout_ms) { + // millis() on both sides: the loop clock is frozen while this blocks. + const uint32_t start = millis(); + while (!this->scan_wanted_ && this->last_result_ == ScanOpResult::PENDING) { + if (millis() - start >= timeout_ms) + return false; + delay(RECONCILE_RETRY_MS); + this->advance_(); + } + return this->last_result_ == ScanOpResult::SETTLED; +} + +// Teardown is asynchronous: the handle is kept until an IDLE observation +// confirms the radio is idle. A rejection WARNs once per failure streak and +// widens the pump gate; the epilogue owns the stuck-teardown deadline. +void BK72xxBLE::release_activity_(BdkActivityState state) { + const BdkOpResult result = + bdk_scan_release(this->scan_activity_idx_, state == BdkActivityState::CREATED, &this->last_release_err_); + if (result == BdkOpResult::OK) { + this->release_warned_ = false; + return; + } + if (!this->release_warned_) { + // A hard error carries its code immediately; the 30 s stuck ERROR follows + // if it persists. + if (result == BdkOpResult::FAILED) { + ESP_LOGW(TAG, "Scan activity release failed (err %d); retrying", this->last_release_err_); + } else { + ESP_LOGW(TAG, "Scan activity release rejected; retrying"); + } + this->release_warned_ = true; + } +} + +// Stamp/track the teardown episode; once past the deadline, ERROR (re-logged +// each interval) and report stuck. +bool BK72xxBLE::teardown_stuck_(uint32_t now) { + if (this->teardown_since_ms_ == 0) { + this->teardown_since_ms_ = now; + this->teardown_stuck_log_ms_ = now; // first ERROR fires at the deadline return false; } - ble_err_t ret = bk_ble_scan_start(this->scan_actv_idx_, &sp, nullptr); - if (ret != ERR_SUCCESS) { - ESP_LOGE(TAG, "Scan start failed (err %d)", static_cast(ret)); - this->scan_actv_idx_ = 0xFF; + if (now - this->teardown_since_ms_ < TEARDOWN_STUCK_ERROR_MS) return false; + if (now - this->teardown_stuck_log_ms_ >= TEARDOWN_STUCK_ERROR_MS) { + if (this->last_release_err_ != 0) { + ESP_LOGE(TAG, "Scan teardown cannot proceed; scanner is stuck (release err %d)", this->last_release_err_); + } else { + // No rejected release this episode: stuck waiting on the controller. + ESP_LOGE(TAG, "Scan teardown cannot proceed; scanner is stuck (controller busy)"); + } + this->teardown_stuck_log_ms_ = now; } return true; } -void BK72xxBLE::scan_stop() { - if (this->scan_actv_idx_ != 0xFF) { - bk_ble_scan_stop(this->scan_actv_idx_, nullptr); - this->scan_actv_idx_ = 0xFF; +// One SDK operation per call toward the latched request; controller state is +// read live each time (it changes on the BLE task, so nothing is mirrored). +// The epilogue owns all deadlines and episode bookkeeping. +ScanOpResult BK72xxBLE::advance_() { + if (!this->scan_wanted_ && this->scan_activity_idx_ == INVALID_ACTIVITY_IDX) { + // Nothing to do; also keeps SDK reads off the pre-enable() path. + this->last_result_ = ScanOpResult::SETTLED; + return ScanOpResult::SETTLED; } + const BdkActivityState state = bdk_scan_state(this->scan_activity_idx_); + const bool ready = bdk_scan_ready(); + ScanOpResult result = this->scan_wanted_ ? this->advance_start_(state, ready) : this->advance_stop_(state, ready); + + const uint32_t now = App.get_loop_component_start_time(); + this->last_advance_ms_ = now; + if (result == ScanOpResult::SETTLED || (state == BdkActivityState::IDLE && ready)) { + // Any teardown episode is over (IDLE observed with the controller + // settled, or e.g. a mode flip that settled back without ever reaching + // IDLE). An IDLE read while an operation is in flight proves nothing — + // a stop deferred there must keep its episode running. + this->reset_teardown_episode_(); + this->release_warned_ = false; + } + if (this->restarting_ && (state == BdkActivityState::IDLE || state == BdkActivityState::CREATED)) { + // The mode-change release is observed complete; the rest is a normal + // bring-up on a fresh budget. + this->restarting_ = false; + this->pending_since_ms_ = now; + } + // Not chained to the clear above: a bring-up waiting at IDLE (create still + // in flight) must keep spending its budget. + if (result == ScanOpResult::PENDING) { + if (this->scan_wanted_ && state != BdkActivityState::STARTED && !this->restarting_) { + // A downed radio spends the bring-up budget; exhausting it hands + // recovery to the tracker's backoff. + if (now - this->pending_since_ms_ >= RECONCILE_PENDING_TIMEOUT_MS) { + ESP_LOGE(TAG, "Scan bring-up did not settle; giving up until the next start"); + result = ScanOpResult::FAILED; + } + } else { + // A teardown is pending: a stop, or a mode-change release still in + // flight (restarting_); either way the bring-up budget waits. + if (this->scan_wanted_) + this->pending_since_ms_ = now; + if (this->teardown_stuck_(now)) { + // Terminal for stop AND restart: the tracker's backoff owns recovery + // (a stop's release keeps re-driving from loop(); a restart is + // re-requested through scan_start() with a fresh deadline). + result = ScanOpResult::FAILED; + } + } + } + this->last_result_ = result; + return result; +} + +ScanOpResult BK72xxBLE::advance_stop_(BdkActivityState state, bool ready) { + if (state == BdkActivityState::IDLE && ready) { + // Fully torn down (or never created): the radio is idle. IDLE is trusted + // only when the controller is settled — mid-create the slot still reads + // IDLE, and dropping the handle then would leak the activity once the + // create lands. + this->scan_activity_idx_ = INVALID_ACTIVITY_IDX; + return ScanOpResult::SETTLED; + } + if (!ready) { + // Acting mid-operation could delete an activity whose start lands + // afterwards, leaking the slot with the radio on; wait. + if (this->last_result_ == ScanOpResult::SETTLED) + ESP_LOGD(TAG, "Scan stop deferred (controller busy)"); + return ScanOpResult::PENDING; + } + // Settled, so CREATED unambiguously means "never started". + this->release_activity_(state); + return ScanOpResult::PENDING; // confirmed once IDLE is observed +} + +ScanOpResult BK72xxBLE::advance_start_(BdkActivityState state, bool ready) { + if (state == BdkActivityState::STARTED) { + if (this->applied_ == this->requested_) + return ScanOpResult::SETTLED; + // Running with different mode or parameters: tear down (the SDK stop + // chain also deletes the activity) and recreate on a later advance. + if (ready) { + this->release_activity_(state); + // Invalidate so a flip back to the old params cannot SETTLE against the + // activity being deleted (interval 0 never matches a real request). + this->applied_.interval = 0; + this->restarting_ = true; + } + return ScanOpResult::PENDING; + } + if (!ready) { + if (this->last_result_ == ScanOpResult::SETTLED) + ESP_LOGD(TAG, "Scan start deferred (controller busy)"); + return ScanOpResult::PENDING; + } + if (state == BdkActivityState::CREATED) { + // Fire-and-forget: SETTLED only once a later advance observes the scan + // running, so a rejected start is retried rather than silently dead. On + // failure the created activity is intact; keep the handle. + if (bdk_scan_start(this->scan_activity_idx_, this->requested_.interval, this->requested_.window, + this->requested_.active) != BdkOpResult::OK) + return ScanOpResult::FAILED; + this->applied_ = this->requested_; + return ScanOpResult::PENDING; + } + if (state == BdkActivityState::OTHER) + return ScanOpResult::PENDING; // transitional; settles on a later read + + // IDLE and ready: acquire a slot and create. A kept index is deliberately + // reused: SDK delete returns the slot to idle and create requires an idle + // slot, so it equals a fresh acquire — while clearing here would orphan a + // create still in flight (the BUSY race below). + if (this->scan_activity_idx_ == INVALID_ACTIVITY_IDX) { + this->scan_activity_idx_ = bdk_scan_acquire_activity(); + if (this->scan_activity_idx_ == INVALID_ACTIVITY_IDX) + return ScanOpResult::FAILED; + } + switch (bdk_scan_create(this->scan_activity_idx_)) { + case BdkOpResult::BUSY: // raced the BLE task; keep the index, the retry resumes this slot + case BdkOpResult::OK: + return ScanOpResult::PENDING; + case BdkOpResult::FAILED: + break; + } + // Safe to clear (unlike BUSY): acquire is a pure search, so a rejected + // create leaves the slot IDLE for re-acquire. + this->scan_activity_idx_ = INVALID_ACTIVITY_IDX; + return ScanOpResult::FAILED; } } // namespace esphome::bk72xx_ble diff --git a/esphome/components/bk72xx_ble/bk72xx_ble.h b/esphome/components/bk72xx_ble/bk72xx_ble.h index 4e615af159..7646f17161 100644 --- a/esphome/components/bk72xx_ble/bk72xx_ble.h +++ b/esphome/components/bk72xx_ble/bk72xx_ble.h @@ -11,6 +11,8 @@ #include +#include "bdk_scan.h" + namespace esphome::bk72xx_ble { enum class BLEComponentState : uint8_t { @@ -19,11 +21,32 @@ enum class BLEComponentState : uint8_t { ACTIVE, }; +/// Outcome of one reconciliation step. +enum class ScanOpResult : uint8_t { + SETTLED, ///< The request is reached: scan observed running, or stopped + ///< with the activity fully released. + PENDING, ///< A step is in flight; loop() keeps advancing — call + ///< scan_start() again to learn the outcome. + FAILED, ///< The controller rejected a step; retry later. +}; + +/// One scan request: mode plus timing, in BLE units (0.625 ms). +struct ScanParams { + bool active; + uint16_t interval; + uint16_t window; + bool operator==(const ScanParams &) const = default; +}; + /// One advertisement report from the controller. struct BLEScanReport { uint8_t mac[6]; // LSB-first, as the controller delivers it int8_t rssi; // signed dBm uint8_t addr_type; + // GAPM report info byte (recv_adv_t.evt_type): bits 0-2 report type + // (1 = legacy adv, 3 = legacy scan response), bit 5 scannable — lets the + // tracker's merger tell the two frames apart. + uint8_t evt_type; uint8_t data_len; // bytes valid in data[] uint8_t data[62]; // legacy advertisement (31) + scan response (31) @@ -69,18 +92,33 @@ class BK72xxBLE final : public Component { void register_scan_listener(BLEScanListener *listener) { this->scan_listeners_.push_back(listener); } #endif - /// Start the controller scan. Interval/window are in BLE units (0.625 ms). - /// Enables the stack first if needed. Returns false on controller failure. - bool scan_start(uint16_t interval, uint16_t window); - /// Stop the controller scan (no-op when not scanning). + /// Request a scan (interval/window in 0.625 ms BLE units); enables the + /// stack first if needed. PENDING until the scan is observed running — + /// loop() keeps advancing, call again to learn the outcome. + ScanOpResult scan_start(uint16_t interval, uint16_t window, bool active); + /// Request the scanner stopped and the activity released; steps that + /// cannot run yet are completed from loop(). void scan_stop(); + /// Drive a requested stop until the radio is observed idle, bounded by + /// timeout_ms (for OTA). Returns false if it still has not settled. + bool flush_pending_stop(uint32_t timeout_ms); + /// Last reconciliation outcome; on FAILED the consumer's retry policy owns + /// recovery. + ScanOpResult last_scan_result() const { return this->last_result_; } /// Internal: buffer one controller report (BDK notice callback, BLE task /// context — bounded copy under the scheduler lock, nothing else). - void enqueue_scan_report(const uint8_t *mac, int8_t rssi, uint8_t addr_type, const uint8_t *data, uint16_t data_len); + void enqueue_scan_report(const uint8_t *mac, int8_t rssi, uint8_t addr_type, uint8_t evt_type, const uint8_t *data, + uint16_t data_len); protected: void resolve_mac_(); + ScanOpResult advance_(); + ScanOpResult advance_stop_(BdkActivityState state, bool ready); + ScanOpResult advance_start_(BdkActivityState state, bool ready); + bool teardown_stuck_(uint32_t now); + void reset_teardown_episode_(); + void release_activity_(BdkActivityState state); #ifdef BK72XX_BLE_SCAN_LISTENER_COUNT // Codegen-sized: no heap allocation, no std::vector template instantiation — @@ -95,10 +133,24 @@ class BK72xxBLE final : public Component { // allocate() returns nullptr before push() can fail. This prevents leaking a // pool slot on a failed push and keeps release() off the producer path. esphome::EventPool report_pool_; - uint8_t ble_mac_[6]{0}; // LSB-first (BLE convention) - uint8_t scan_actv_idx_{0xFF}; - BLEComponentState state_{BLEComponentState::STATE_OFF}; + // Largest-to-smallest: padding only at the tail, absorbed by future byte fields. + uint32_t last_advance_ms_{0}; + uint32_t pending_since_ms_{0}; // bring-up budget anchor; refilled on request change + uint32_t teardown_since_ms_{0}; // unfinished teardown episode start; 0 = none + uint32_t teardown_stuck_log_ms_{0}; // last stuck-teardown ERROR; re-logged each TEARDOWN_STUCK_ERROR_MS + int last_release_err_{0}; // SDK code of the episode's last failed release; 0 = none + ScanParams requested_{}; // latched by scan_start() + ScanParams applied_{}; // last params we commanded; mismatch with requested_ restarts + uint8_t ble_mac_[6]{0}; // LSB-first (BLE convention) + uint8_t scan_activity_idx_{INVALID_ACTIVITY_IDX}; + bool scan_wanted_{false}; // the latched request is to scan (vs stopped) + bool release_warned_{false}; // gates the release WARN; widens the pump gate + bool restarting_{false}; // mode-change release in flight; teardown deadline governs until released bool enable_on_boot_{false}; + // PENDING means advance_() has more to do; loop() drives it, paced and + // (for a bring-up) bounded. + ScanOpResult last_result_{ScanOpResult::SETTLED}; + BLEComponentState state_{BLEComponentState::STATE_OFF}; }; } // namespace esphome::bk72xx_ble diff --git a/esphome/components/bk72xx_ble_tracker/__init__.py b/esphome/components/bk72xx_ble_tracker/__init__.py index 7fefb310cd..96b3536601 100644 --- a/esphome/components/bk72xx_ble_tracker/__init__.py +++ b/esphome/components/bk72xx_ble_tracker/__init__.py @@ -25,6 +25,7 @@ from esphome.components.ble_device_base import automation as ble_automation from esphome.components.const import CONF_ON_SCAN_END, CONF_SCAN_PARAMETERS, CONF_WINDOW import esphome.config_validation as cv from esphome.const import ( + CONF_ACTIVE, CONF_CONTINUOUS, CONF_DURATION, CONF_ID, @@ -146,6 +147,9 @@ async def stop_scan_action_to_code( async def to_code(config: ConfigType) -> None: # Selects the BLEHub alias arm in ble_device_base/ble_hub_impl.h. cg.add_define("USE_BK72XX_BLE_TRACKER") + # Compiles the shared adv + scan-response merge (the BDK delivers the pair + # as separate reports). + cg.add_define("USE_BLE_SCAN_RESPONSE_MERGER") var = cg.new_Pvariable(config[CONF_ID]) await cg.register_component(var, config) @@ -164,6 +168,7 @@ async def to_code(config: ConfigType) -> None: cg.add(var.set_scan_window(ble_device_base.to_ble_units(scan[CONF_WINDOW]))) cg.add(var.set_scan_duration(scan[CONF_DURATION].total_milliseconds)) cg.add(var.set_configured_continuous(scan[CONF_CONTINUOUS])) + cg.add(var.set_scan_active(scan[CONF_ACTIVE])) for conf in config.get(CONF_ON_BLE_ADVERTISE, []): await ble_automation.advertise_trigger_to_code(conf, var) diff --git a/esphome/components/bk72xx_ble_tracker/bk72xx_ble_tracker.cpp b/esphome/components/bk72xx_ble_tracker/bk72xx_ble_tracker.cpp index a58561f2de..a312d2496f 100644 --- a/esphome/components/bk72xx_ble_tracker/bk72xx_ble_tracker.cpp +++ b/esphome/components/bk72xx_ble_tracker/bk72xx_ble_tracker.cpp @@ -9,10 +9,9 @@ #include "bk72xx_ble_tracker.h" -#include #include -#include "esphome/core/hal.h" +#include "esphome/core/application.h" #include "esphome/core/log.h" namespace esphome::bk72xx_ble_tracker { @@ -27,6 +26,15 @@ static const char *const TAG = "bk72xx_ble_tracker"; // a single WARN is emitted when the retry interval first saturates. static constexpr uint32_t SCAN_START_RETRY_MS = 1000; static constexpr uint8_t SCAN_START_RETRY_MAX_DOUBLINGS = 6; // 1 s << 6 = 64 s +// Stable-run time before the failure streak clears; reset-on-start would keep +// a flapping controller at the 1 s gate. +static constexpr uint32_t SCAN_STABLE_RESET_MS = 30000; + +// Radio-idle deadline for the bounded stop drain at OTA start. +static constexpr uint32_t OTA_STOP_FLUSH_MS = 100; + +// 0.625 ms BLE units; integer math avoids soft-float on this FPU-less part. +constexpr uint32_t ble_units_to_ms(uint32_t units) { return units * 5 / 8; } // --------------------------------------------------------------------------- // Component lifecycle @@ -36,11 +44,20 @@ void BK72xxBLETracker::setup() { // Receive the controller's scan reports; the controller queues them from the // BLE task and delivers here on the main task. this->parent_->register_scan_listener(this); + // Merged (and unmerged) frames go to the shared dispatcher; unclaimed + // devices are logged only on one-shot scans (continuous would spam). + this->merger_.bind(&this->dispatcher_, &this->scan_continuous_, TAG); #ifdef USE_OTA_STATE_LISTENER // Pause scanning while an OTA update is in flight — on the single-core BK72xx the // BLE scan competes with the OTA flash writes. Mirrors esp32_ble_tracker. ota::get_global_ota_callback()->add_global_state_listener(this); #endif + // scan_requested_ check: an on_boot start_scan latched before this setup() + // must keep the retry loop running (rp2/ln882h parity). + if (!this->scan_continuous_ && !this->scan_requested_) { + // Nothing to time until an explicit start_scan(); it re-enables the loop. + this->disable_loop(); + } } #ifdef USE_OTA_STATE_LISTENER @@ -50,30 +67,54 @@ void BK72xxBLETracker::on_ota_global_state(ota::OTAState state, float progress, this->scan_continuous_before_ota_ = this->scan_continuous_; this->scan_requested_before_ota_ = this->scan_requested_; this->stop_scan(); + // The transfer starves the loop; a deferred stop would leave the radio + // scanning for the whole update, so drain it here, bounded. + if (!this->parent_->flush_pending_stop(OTA_STOP_FLUSH_MS)) + ESP_LOGE(TAG, "Scan still stopping at OTA start; the radio may contend with the update"); } else if (state == ota::OTA_ERROR || state == ota::OTA_ABORT) { // On success the device reboots, so restore only on a failed/aborted update; // loop() restarts the scan on its next iteration (continuous idle branch). if (this->scan_continuous_before_ota_) { this->scan_continuous_before_ota_ = false; this->scan_continuous_ = true; + this->enable_loop(); // stop_scan() parked it } // A one-shot request that was still pending (latched, retrying) when the // OTA paused scanning is re-latched, not dropped — loop() resumes the retry. if (this->scan_requested_before_ota_) { this->scan_requested_before_ota_ = false; this->scan_requested_ = true; + this->enable_loop(); } } } #endif // USE_OTA_STATE_LISTENER void BK72xxBLETracker::loop() { - const uint32_t now = millis(); + const uint32_t now = App.get_loop_component_start_time(); + + // Deliver held scannable advertisements whose scan response never arrived — + // unmerged after the merger's timeout. + if (!this->merger_.empty()) + this->merger_.sweep(now); + + // Before the drop branch: a drop after a stable run starts a fresh streak. + if (this->scan_running_ && this->failed_start_count_ != 0 && now - this->scan_start_time_ >= SCAN_STABLE_RESET_MS) + this->failed_start_count_ = 0; + + // A terminal failure while we report running recovers via the normal retry + // path; the drop charges the backoff so a flapping controller escalates. + if (this->scan_running_ && this->parent_->last_scan_result() == bk72xx_ble::ScanOpResult::FAILED) { + ESP_LOGW(TAG, "Controller scan lost; retrying"); + this->scan_requested_ = true; + this->count_failed_start_(); + this->mark_scan_ended_(now); + } + if (this->scan_continuous_) { if (!this->scan_running_) { - // A start that succeeded re-anchored the period timer from a later millis(), - // so the stale `now` below would underflow the comparison and fire - // on_scan_end() for a scan that just began. Resume next iteration. + // One-iteration deferral; all stamps share this iteration's cached + // timestamp, so the period check below cannot underflow. if (this->try_start_with_backoff_(now)) return; } @@ -81,11 +122,7 @@ void BK72xxBLETracker::loop() { // esp32_ble_tracker::cleanup_scan_state_(). Gated on scan_started_once_ so a scan // that never came up (start kept failing) does not fire spurious on_scan_end events. if (this->scan_started_once_ && now - this->scan_period_start_ >= this->scan_duration_) { -#ifdef ESPHOME_BLE_DEVICE_BASE_LISTENER_COUNT - for (auto *listener : this->listeners_) - listener->on_scan_end(); - this->discovered_log_.clear(); // reset per-scan "Found device" dedup (esp32_ble_tracker parity) -#endif + this->fire_scan_end_(); this->scan_period_start_ = now; } return; @@ -99,13 +136,14 @@ void BK72xxBLETracker::loop() { // would be silent: the scan never runs, stop_scan_() is never reached and // on_scan_end() never fires, leaving period-keyed consumers waiting forever. if (this->scan_requested_ && !this->scan_running_) { - // Same stale-`now` hazard as the continuous branch: start_scan_() stamps - // scan_start_time_ from a later millis(), so the duration check below would - // underflow and stop the scan in the iteration that started it. + // Same one-iteration deferral as the continuous branch. if (this->try_start_with_backoff_(now)) return; } if (this->scan_running_ && now - this->scan_start_time_ >= this->scan_duration_) { + // A full-duration run proves the controller healthy even when duration is + // shorter than SCAN_STABLE_RESET_MS. + this->failed_start_count_ = 0; this->stop_scan_(); } } @@ -122,32 +160,54 @@ bool BK72xxBLETracker::try_start_with_backoff_(uint32_t now, bool force) { // even user-initiated attempts respect the backoff, so a start_scan() action // on a short cadence cannot hammer a failing controller; the attempt stays // inside the failure accounting below either way. - const uint8_t doublings = std::min(this->failed_start_count_, SCAN_START_RETRY_MAX_DOUBLINGS); - if ((!force || this->failed_start_count_ != 0) && - now - this->last_scan_start_attempt_ < (SCAN_START_RETRY_MS << doublings)) + // Mid bring-up, observe instead of re-issuing (the hub self-advances). A + // SETTLED outcome completes immediately; only fresh attempts after FAILED + // are rate-limited. + const auto hub = this->parent_->last_scan_result(); + if (hub == bk72xx_ble::ScanOpResult::PENDING) return false; - this->last_scan_start_attempt_ = now; + if (hub == bk72xx_ble::ScanOpResult::FAILED) { + if (this->start_attempt_open_) { + // Our bring-up gave up asynchronously; charge it to the backoff. + this->start_attempt_open_ = false; + this->count_failed_start_(); + } + if ((!force || this->failed_start_count_ != 0) && + now - this->last_scan_start_attempt_ < (SCAN_START_RETRY_MS << this->failed_start_count_)) + return false; + } this->start_scan_(); - if (!this->scan_running_ && this->failed_start_count_ < SCAN_START_RETRY_MAX_DOUBLINGS) { + if (!this->scan_running_) { + if (this->parent_->last_scan_result() == bk72xx_ble::ScanOpResult::PENDING) { + this->start_attempt_open_ = true; + return false; // the controller is still bringing the scan up; not a failure + } + this->count_failed_start_(); + } + return this->scan_running_; +} + +void BK72xxBLETracker::count_failed_start_() { + if (this->failed_start_count_ < SCAN_START_RETRY_MAX_DOUBLINGS) { ++this->failed_start_count_; if (this->failed_start_count_ == SCAN_START_RETRY_MAX_DOUBLINGS) { ESP_LOGW(TAG, "Scan start keeps failing; retrying every %" PRIu32 " s", (SCAN_START_RETRY_MS << SCAN_START_RETRY_MAX_DOUBLINGS) / 1000); } } - return this->scan_running_; } void BK72xxBLETracker::dump_config() { ESP_LOGCONFIG(TAG, "BK72xx BLE Tracker:\n" " Scan Duration: %" PRIu32 " s\n" - " Scan Interval: %.0f ms (%" PRIu32 " BLE units)\n" - " Scan Window: %.0f ms (%" PRIu32 " BLE units)\n" - " Scan Type: PASSIVE\n" + " Scan Interval: %" PRIu32 " ms (%" PRIu32 " BLE units)\n" + " Scan Window: %" PRIu32 " ms (%" PRIu32 " BLE units)\n" + " Scan Type: %s (configured %s)\n" " Continuous Scanning: %s", - this->scan_duration_ / 1000, this->scan_interval_ * 0.625f, this->scan_interval_, - this->scan_window_ * 0.625f, this->scan_window_, YESNO(this->scan_continuous_)); + this->scan_duration_ / 1000, ble_units_to_ms(this->scan_interval_), this->scan_interval_, + ble_units_to_ms(this->scan_window_), this->scan_window_, this->scan_active_ ? "ACTIVE" : "PASSIVE", + this->scan_active_configured_ ? "ACTIVE" : "PASSIVE", YESNO(this->scan_continuous_)); } // --------------------------------------------------------------------------- @@ -156,31 +216,33 @@ void BK72xxBLETracker::dump_config() { // listener dispatch run in main-loop context with no cross-task handling here. // --------------------------------------------------------------------------- -void BK72xxBLETracker::on_scan_report(const bk72xx_ble::BLEScanReport &report) { - // Raw callback (the raw-advertisement path). - if (this->raw_advertisement_callback_.is_set()) { - const ble_device_base::RawAdvertisement adv{.address = ble_device_base::mac_lsb_first_to_uint64(report.mac), - .data = report.data, - .data_len = report.data_len, - .rssi = report.rssi, - .addr_type = report.addr_type}; - this->raw_advertisement_callback_.invoke(adv); - } +// GAPM report info byte (BLEScanReport::evt_type): bits 0-2 report type, +// bit 5 scannable advertisement. Verified against both BDK stacks (5.1 and +// 5.2 fill it from gapm_ext_adv_report_ind.info). +static constexpr uint8_t GAPM_REPORT_TYPE_MASK = 0x07; +static constexpr uint8_t GAPM_REPORT_TYPE_SCAN_RSP_EXT = 2; +static constexpr uint8_t GAPM_REPORT_TYPE_SCAN_RSP_LEG = 3; +static constexpr uint8_t GAPM_REPORT_INFO_SCAN_ADV_BIT = 1 << 5; -#ifdef ESPHOME_BLE_DEVICE_BASE_LISTENER_COUNT - ble_device_base::ESPBTDevice device; - device.from_scan_result(report.mac, report.rssi, report.addr_type, report.data, report.data_len); - bool found = false; - for (auto *listener : this->listeners_) { - if (listener->parse_device(device)) { - found = true; - } +// Demux advertisements vs scan responses into the shared merger: the BDK +// delivers the pair as separate reports; a scannable advertisement is held +// until its scan response arrives and delivered as one merged frame. +void BK72xxBLETracker::on_scan_report(const bk72xx_ble::BLEScanReport &report) { + const uint8_t rtype = report.evt_type & GAPM_REPORT_TYPE_MASK; + if (rtype == GAPM_REPORT_TYPE_SCAN_RSP_LEG || rtype == GAPM_REPORT_TYPE_SCAN_RSP_EXT) { + this->merger_.submit_scan_rsp(report.mac, report.rssi, report.addr_type, report.data, report.data_len); + return; } - // Mirror esp32_ble_tracker: log a newly-seen device only when nothing claimed - // it and the scan is one-shot (continuous scans would spam). - if (!found && !this->scan_continuous_) - this->discovered_log_.log_device(TAG, device); -#endif // ESPHOME_BLE_DEVICE_BASE_LISTENER_COUNT + // Stash only while an active scan runs: a passive scan never gets a + // response, and after a stop nothing would sweep the merger, so a late + // report would surface minutes later as a fresh advertisement. + if (this->scan_running_ && this->scan_active_ && (report.evt_type & GAPM_REPORT_INFO_SCAN_ADV_BIT)) { + this->merger_.stash_adv(report.mac, report.rssi, report.addr_type, report.data, report.data_len, + App.get_loop_component_start_time()); + return; + } + this->dispatcher_.dispatch(report.mac, report.rssi, report.addr_type, report.data, report.data_len, + /*raw_only=*/false, this->scan_continuous_ ? nullptr : TAG); } // --------------------------------------------------------------------------- @@ -207,7 +269,8 @@ void BK72xxBLETracker::start_scan() { // against a failing controller, repeated start_scan() calls are rate-limited // like any other attempt. this->scan_requested_ = true; - this->try_start_with_backoff_(millis(), /* force= */ true); + this->enable_loop(); // an idle one-shot tracker parked it in stop_scan_() + this->try_start_with_backoff_(App.get_loop_component_start_time(), /* force= */ true); } void BK72xxBLETracker::restart_scan_duration() { @@ -218,7 +281,7 @@ void BK72xxBLETracker::restart_scan_duration() { // start_scan action fired more often than scan_duration_ would otherwise // suppress on_scan_end indefinitely — and absence detection (ble_rssi's NAN // publish) rides on that period. - this->scan_start_time_ = millis(); + this->scan_start_time_ = App.get_loop_component_start_time(); } void BK72xxBLETracker::stop_scan() { @@ -231,24 +294,31 @@ void BK72xxBLETracker::stop_scan() { // Internal scan start / stop // --------------------------------------------------------------------------- +bk72xx_ble::ScanOpResult BK72xxBLETracker::controller_scan_start_() { + this->last_scan_start_attempt_ = App.get_loop_component_start_time(); + return this->parent_->scan_start(static_cast(this->scan_interval_), + static_cast(this->scan_window_), this->scan_active_); +} + void BK72xxBLETracker::start_scan_() { if (this->scan_running_) return; - if (!this->parent_->scan_start(static_cast(this->scan_interval_), - static_cast(this->scan_window_))) + if (this->controller_scan_start_() != bk72xx_ble::ScanOpResult::SETTLED) return; - const uint32_t now = millis(); + const uint32_t now = App.get_loop_component_start_time(); this->scan_running_ = true; this->scan_requested_ = false; // the latched one-shot request is satisfied - this->failed_start_count_ = 0; // reset here so direct starts clear the backoff too + this->start_attempt_open_ = false; + // failed_start_count_ deliberately not reset here; only a stable run clears it (loop()). this->scan_start_time_ = now; // Log every explicit start at DEBUG — stop_scan_() logs every stop at DEBUG, and // in non-continuous mode each period is an explicit start, so asymmetric logging // would read as the scanner failing to come back up. - ESP_LOGD(TAG, "Scan started (passive, window=%.0fms, interval=%.0fms)", this->scan_window_ * 0.625f, - this->scan_interval_ * 0.625f); + ESP_LOGD(TAG, "Scan started (%s, window=%" PRIu32 "ms, interval=%" PRIu32 "ms)", + this->scan_active_ ? "active" : "passive", ble_units_to_ms(this->scan_window_), + ble_units_to_ms(this->scan_interval_)); // Re-anchor the on_scan_end period to every successful start — first start (so the // period counts from the scan, not from boot) and every restart after a stop (so // resuming after longer than scan_duration, e.g. a failed OTA restoring continuous @@ -258,18 +328,48 @@ void BK72xxBLETracker::start_scan_() { this->scan_started_once_ = true; } +// Deliberate logical/physical split: on_scan_end() reports the tracker's +// intent while the hub winds the radio down asynchronously; OTA is the one +// path that must wait, and it flushes explicitly. void BK72xxBLETracker::stop_scan_() { - if (!this->scan_running_) - return; - this->parent_->scan_stop(); + this->start_attempt_open_ = false; // an abandoned bring-up is not charged + this->parent_->scan_stop(); // idempotent: releases whatever the hub holds + if (this->scan_running_) { + ESP_LOGD(TAG, "Scan stopped"); + this->mark_scan_ended_(App.get_loop_component_start_time()); + } + // Park when idle (the hub drives its own teardown); re-check because an + // on_scan_end automation may have restarted the scan. + if (!this->scan_continuous_ && !this->scan_running_ && !this->scan_requested_) + this->disable_loop(); +} + +// The period re-anchor keeps on_scan_end from double-firing in one iteration. +void BK72xxBLETracker::mark_scan_ended_(uint32_t now) { this->scan_running_ = false; - ESP_LOGD(TAG, "Scan stopped"); -#ifdef ESPHOME_BLE_DEVICE_BASE_LISTENER_COUNT - for (auto *listener : this->listeners_) - listener->on_scan_end(); - this->discovered_log_.clear(); // reset per-scan "Found device" dedup (esp32_ble_tracker parity) -#endif - this->scan_period_start_ = millis(); // reset period clock so on_scan_end does not double-fire + this->fire_scan_end_(); + this->scan_period_start_ = now; +} + +void BK72xxBLETracker::fire_scan_end_() { + // Deliver held advertisements whose scan response never came (unmerged) + // BEFORE on_scan_end fires. + this->merger_.flush(); + this->dispatcher_.on_scan_end(); +} + +// true = request latched, not applied: the reconciler applies it +// asynchronously and loop() recovers a failed re-arm (ln882h parity). +bool BK72xxBLETracker::request_scan_mode(bool active) { + if (this->scan_active_ == active) + return true; + this->scan_active_ = active; + ESP_LOGD(TAG, "Scan mode %s", active ? "active" : "passive"); + // The controller reconciler restarts a running scan itself; the scan stays + // logically running. An idle scanner picks the mode up on its next start. + if (this->scan_running_) + this->controller_scan_start_(); + return true; } } // namespace esphome::bk72xx_ble_tracker diff --git a/esphome/components/bk72xx_ble_tracker/bk72xx_ble_tracker.h b/esphome/components/bk72xx_ble_tracker/bk72xx_ble_tracker.h index cc51918da5..59d17f9b84 100644 --- a/esphome/components/bk72xx_ble_tracker/bk72xx_ble_tracker.h +++ b/esphome/components/bk72xx_ble_tracker/bk72xx_ble_tracker.h @@ -21,6 +21,7 @@ // window: 30ms // duration: 5min // continuous: true +// active: true #pragma once @@ -29,6 +30,7 @@ #include "esphome/components/bk72xx_ble/bk72xx_ble.h" #include "esphome/components/ble_device_base/ble_device.h" #include "esphome/components/ble_device_base/ble_hub.h" +#include "esphome/components/ble_device_base/scan_response_merger.h" #include "esphome/core/component.h" #include "esphome/core/helpers.h" @@ -69,6 +71,12 @@ class BK72xxBLETracker : public Component, void set_scan_interval(uint32_t scan_interval) { this->scan_interval_ = scan_interval; } void set_scan_window(uint32_t scan_window) { this->scan_window_ = scan_window; } void set_scan_duration(uint32_t scan_duration) { this->scan_duration_ = scan_duration; } + /// Set from YAML (scan_parameters.active); runtime mode requests change + /// only the resolved mode. + void set_scan_active(bool scan_active) { + this->scan_active_ = scan_active; + this->scan_active_configured_ = scan_active; + } /// Set from YAML (scan_parameters.continuous); also the value /// configured_continuous() reports and a bare start_scan action restores. void set_configured_continuous(bool scan_continuous) { @@ -93,27 +101,19 @@ class BK72xxBLETracker : public Component, // ---- ble_device_base::BLEHub contract ---- void register_listener(ble_device_base::ESPBTDeviceListener *listener) { -#ifdef ESPHOME_BLE_DEVICE_BASE_LISTENER_COUNT - this->listeners_.push_back(listener); -#endif + this->dispatcher_.register_listener(listener); } void set_raw_advertisement_callback(ble_device_base::RawAdvertisementCallback callback) { - this->raw_advertisement_callback_ = callback; + this->dispatcher_.set_raw_advertisement_callback(callback); } static constexpr ble_device_base::HubCapabilities get_capabilities() { - // The Beken BDK exposes no active-scan path (passive scanning only), so the - // controller never solicits scan responses and never merges them; consumers - // relying on scan-response fields (device names) get them only where the - // receiver merges per address (Home Assistant does). No GATT client either. - // scan_mode_switch stays false for the same reason: with no active-scan - // path there is no mode to switch to. - return {.active_scan = false, .merges_scan_response = false, .gatt = false, .scan_mode_switch = false}; - } - bool request_scan_mode(bool active) { - // Passive-only controller: a passive request is already honored, an active - // one cannot be. - return !active; + // Active scanning is driven through bk72xx_ble's reconciler because the BDK + // API itself is passive-only. The controller delivers scan responses as + // separate reports; this tracker merges the pair before delivery (shared + // ScanResponseMerger, Bluedroid semantics). No GATT client. + return {.active_scan = true, .merges_scan_response = true, .gatt = false, .scan_mode_switch = true}; } + bool request_scan_mode(bool active); // The controller stores the address LSB-first (BLE convention); the contract // wants printable (MSB-first) order. void get_adapter_mac(uint8_t out[6]) { @@ -123,7 +123,7 @@ class BK72xxBLETracker : public Component, out[i] = mac[5 - i]; } bool scan_running() { return this->scan_running_; } - bool scan_active() { return false; } // BK72xx scan is passive-only + bool scan_active() { return this->scan_active_; } // ---- bk72xx_ble::BLEScanListener ---- // Delivered by the controller's loop() on the ESPHome main task — the @@ -133,15 +133,20 @@ class BK72xxBLETracker : public Component, protected: void start_scan_(); void stop_scan_(); - /// Attempt a rate-limited (re)start; returns true when the scan is running, - /// which means the caller must not compare its cached millis() against the - /// timestamps start_scan_() just refreshed. force bypasses the rate gate for - /// an explicit user start only while the failure streak is clean; a failing - /// controller rate-limits forced attempts too. Failure accounting always runs. + void fire_scan_end_(); + void mark_scan_ended_(uint32_t now); + /// Stamp-and-start for every controller scan attempt, so the retry rate + /// limit covers all callers. + bk72xx_ble::ScanOpResult controller_scan_start_(); + /// Rate-limited (re)start; true when the scan is running (the caller must + /// not reuse a `now` older than the stamps this refreshed). Force and + /// backoff rules are documented at the definition. bool try_start_with_backoff_(uint32_t now, bool force = false); + void count_failed_start_(); bool scan_running_{false}; - bool scan_requested_{false}; // latched start_scan() request not yet running; loop() retries with backoff + bool scan_requested_{false}; // latched start_scan() request not yet running; loop() retries with backoff + bool start_attempt_open_{false}; // charge a later FAILED observation to the backoff exactly once // Defaults: the BK reference — 30 % duty cycle // (interval 100 ms / window 30 ms), in 0.625 ms BLE units. uint32_t scan_interval_{160}; // 160 × 0.625 ms = 100 ms @@ -149,30 +154,27 @@ class BK72xxBLETracker : public Component, uint32_t scan_duration_{300000}; bool scan_continuous_{true}; bool scan_continuous_configured_{true}; // YAML value; stop_scan() must not lose it + bool scan_active_{true}; // resolved mode; see scan_parameters.active + bool scan_active_configured_{true}; // YAML value; runtime requests must not lose it #ifdef USE_OTA_STATE_LISTENER bool scan_continuous_before_ota_{false}; // continuous mode saved at OTA start, restored on OTA failure bool scan_requested_before_ota_{false}; // pending one-shot latch saved at OTA start, re-latched on OTA failure #endif uint32_t scan_start_time_{0}; - uint32_t last_scan_start_attempt_{0}; // millis() of last start_scan_() attempt; rate-limits retries - uint8_t failed_start_count_{0}; // consecutive failed starts; drives the retry backoff (reset on success) - uint32_t scan_period_start_{0}; // millis() at start of current scan period; used to rate-limit on_scan_end() + uint32_t last_scan_start_attempt_{0}; // last controller start attempt, any caller; rate-limits retries + uint8_t failed_start_count_{0}; // failed starts AND drops; backoff shift, cleared after a stable run (loop()) + uint32_t scan_period_start_{0}; // loop-clock start of the scan period; rate-limits on_scan_end() bool scan_started_once_{false}; // true after first successful scan start; gates the period timer - ble_device_base::RawAdvertisementCallback raw_advertisement_callback_{}; -#ifdef ESPHOME_BLE_DEVICE_BASE_LISTENER_COUNT - // Parsed-advertisement consumers registered through ble_device_base. - // Codegen-sized: no heap allocation, no std::vector template instantiations. - StaticVector listeners_; -#endif - -#ifdef ESPHOME_BLE_DEVICE_BASE_LISTENER_COUNT - // Per-period "Found device" DEBUG log with MAC dedup — shared implementation - // in ble_device_base, identical output on every tracker backend. Guarded like - // its only writer so a no-listener build does not carry an unused vector. - ble_device_base::DiscoveredDeviceLog discovered_log_{}; -#endif + // Shared adv + scan-response merge and frame dispatch (ble_device_base). + // All calls run on the main task (the controller queue already crossed + // tasks). Merger clock: stash_adv() reads the PARENT's cached loop time + // (on_scan_report runs inside bk72xx_ble's queue drain), sweep() this + // component's — same App.loop() pass, so the delta stays non-negative and + // the 300 ms timeout holds. + ble_device_base::ScanResponseMerger merger_; + ble_device_base::AdvDispatcher dispatcher_; }; } // namespace esphome::bk72xx_ble_tracker diff --git a/esphome/components/ble_device_base/__init__.py b/esphome/components/ble_device_base/__init__.py index ae03003713..4da7d48882 100644 --- a/esphome/components/ble_device_base/__init__.py +++ b/esphome/components/ble_device_base/__init__.py @@ -247,24 +247,23 @@ def scan_parameters_schema( interval_default: str, *, window_default: str = "30ms", - supports_active: bool = False, ) -> cv.All: """Build the scan_parameters value schema shared by all BLE trackers. interval_default and window_default are per chip (e.g. esp32 320/30 ms, bk72xx/rp2 100/30 ms — the reference scan rates of the respective stacks; - LN882H's SDK recommends 100/50 ms). Pass supports_active=True only when - the tracker supports active scanning; it exposes the `active` option - (whose own default is on, esp32_ble_tracker behavior). + LN882H's SDK recommends 100/50 ms). The `active` option (default on) is + unconditional: active scanning is part of the tracker contract — every + current proxy client assumes it, so a passive-only tracker must not share + this schema. """ schema = { cv.Optional(CONF_DURATION, default="5min"): cv.positive_time_period_seconds, cv.Optional(CONF_INTERVAL, default=interval_default): cv.positive_time_period, cv.Optional(CONF_WINDOW, default=window_default): cv.positive_time_period, cv.Optional(CONF_CONTINUOUS, default=True): cv.boolean, + cv.Optional(CONF_ACTIVE, default=True): cv.boolean, } - if supports_active: - schema[cv.Optional(CONF_ACTIVE, default=True)] = cv.boolean return cv.All(cv.Schema(schema), validate_scan_parameters) diff --git a/esphome/components/ble_device_base/ble_hub.h b/esphome/components/ble_device_base/ble_hub.h index 8e4c710bb1..9da6371012 100644 --- a/esphome/components/ble_device_base/ble_hub.h +++ b/esphome/components/ble_device_base/ble_hub.h @@ -83,9 +83,9 @@ struct HubCapabilities { /// Today: esp32 and rp2. bool gatt; /// request_scan_mode() is honored at runtime. Distinct from active_scan: - /// a passive-only controller (bk72xx) can never switch, and a hub may - /// support active scanning yet still refuse the runtime switch - /// (esp32_ble_tracker drives its mode through its own tracker API). + /// a passive-only controller can never switch, and a hub may support + /// active scanning yet still refuse the runtime switch (esp32_ble_tracker + /// drives its mode through its own tracker API). bool scan_mode_switch; }; diff --git a/esphome/components/ble_device_base/scan_response_merger.cpp b/esphome/components/ble_device_base/scan_response_merger.cpp index 15445cee02..2dd1fd6927 100644 --- a/esphome/components/ble_device_base/scan_response_merger.cpp +++ b/esphome/components/ble_device_base/scan_response_merger.cpp @@ -8,7 +8,8 @@ namespace esphome::ble_device_base { void ScanResponseMerger::deliver_(const uint8_t *mac, int8_t rssi, uint8_t addr_type, const uint8_t *data, uint8_t data_len, bool raw_only) { - if (this->dispatcher_ == nullptr) + // A partial bind is treated as unbound; never dereference half a binding. + if (this->dispatcher_ == nullptr || this->scan_continuous_ == nullptr) return; this->dispatcher_->dispatch(mac, rssi, addr_type, data, data_len, raw_only, *this->scan_continuous_ ? nullptr : this->log_tag_); diff --git a/esphome/components/bluetooth_connection/__init__.py b/esphome/components/bluetooth_connection/__init__.py index 6ddd1ab9e1..38c9649aa1 100644 --- a/esphome/components/bluetooth_connection/__init__.py +++ b/esphome/components/bluetooth_connection/__init__.py @@ -200,6 +200,7 @@ SOURCE_FILE_FRAMEWORKS: dict[str, set[PlatformFramework]] = { "bluetooth_connection_hub.cpp": { PlatformFramework.RP2_ARDUINO, PlatformFramework.LN882X_ARDUINO, + PlatformFramework.BK72XX_ARDUINO, PlatformFramework.ESP32_ARDUINO, PlatformFramework.ESP32_IDF, }, diff --git a/esphome/components/bluetooth_proxy/__init__.py b/esphome/components/bluetooth_proxy/__init__.py index 36a4594c33..6a1c874170 100644 --- a/esphome/components/bluetooth_proxy/__init__.py +++ b/esphome/components/bluetooth_proxy/__init__.py @@ -7,6 +7,7 @@ import esphome.config_validation as cv from esphome.const import ( CONF_ACTIVE, CONF_ID, + PLATFORM_BK72XX, PLATFORM_ESP32, PLATFORM_LN882X, PLATFORM_RP2, @@ -47,15 +48,13 @@ def AUTO_LOAD(config: ConfigType | None = None) -> list[str]: # Platforms with an in-tree ble_device_base BLE tracker hub whose controller -# supports active scanning. Passive-only hubs (bk72xx) are deliberately NOT -# admitted yet: every current client (aioesphomeapi, bleak-esphome, Home -# Assistant) assumes an ESPHome proxy can scan actively, so a passive-only -# proxy would be misdriven — bk72xx follows once the API carries a feature -# flag clients can trust (FEATURE_ACTIVE_SCAN + a version flag, separate PRs). -# Coupled to bluetooth_connection: platforms here are also listed in its -# _PLATFORM_BACKENDS registry, HUB_MAX_CONNECTIONS, and FILTER_SOURCE_FILES -# hub entry. -_HUB_PLATFORMS = (PLATFORM_LN882X, PLATFORM_RP2) +# supports active scanning — every current client (aioesphomeapi, bleak-esphome, +# Home Assistant) assumes an ESPHome proxy can scan actively, so a passive-only +# hub must not be admitted (it would be misdriven). +# Coupled to bluetooth_connection: platforms with a GATT backend are also +# listed in its _PLATFORM_BACKENDS registry, HUB_MAX_CONNECTIONS, and +# FILTER_SOURCE_FILES hub entry. +_HUB_PLATFORMS = (PLATFORM_BK72XX, PLATFORM_LN882X, PLATFORM_RP2) DEPENDENCIES = ["api"] CODEOWNERS = ["@jesserockz", "@bdraco"] @@ -265,11 +264,15 @@ def _validate_platform(config: ConfigType) -> ConfigType: # Fail here with the actual reason. Without this gate the error surfaces # later as an unresolvable hub ID ("Are you missing a hub declaration?") # on platforms where no hub component can be declared. + full = ", ".join(["esp32", *sorted(bluetooth_connection.HUB_MAX_CONNECTIONS)]) + adv_only = ", ".join( + sorted(set(_HUB_PLATFORMS) - set(bluetooth_connection.HUB_MAX_CONNECTIONS)) + ) raise cv.Invalid( f"bluetooth_proxy is not supported on {CORE.target_platform}: no " "active-scan-capable BLE tracker hub is available for this " - "platform. It runs on esp32 and rp2 (full proxy) and the ln882x " - "family (advertisement-only)." + f"platform. It runs on {full} (full proxy) and {adv_only} " + "(advertisement-only)." ) if CORE.target_platform in bluetooth_connection.HUB_MAX_CONNECTIONS: return _GATT_HUB_SCHEMAS[CORE.target_platform]()(config) diff --git a/esphome/components/bluetooth_proxy/bluetooth_proxy.cpp b/esphome/components/bluetooth_proxy/bluetooth_proxy.cpp index 0a16567549..af52a25ec0 100644 --- a/esphome/components/bluetooth_proxy/bluetooth_proxy.cpp +++ b/esphome/components/bluetooth_proxy/bluetooth_proxy.cpp @@ -198,6 +198,10 @@ void BluetoothProxy::reset_connection_slot_(BluetoothConnection *connection, con } BluetoothConnection *BluetoothProxy::get_connection_(uint64_t address, bool reserve) { + // Finish the scan before reserving: a free slot earlier in the array must + // not win over a later slot that already holds the address, or one device + // ends up on two slots with a second connection attempt racing the first. + BluetoothConnection *free_slot = nullptr; for (uint8_t i = 0; i < this->connection_count_; i++) { auto *connection = this->connections_[i]; uint64_t conn_addr = connection->get_address(); @@ -205,18 +209,19 @@ BluetoothConnection *BluetoothProxy::get_connection_(uint64_t address, bool rese if (conn_addr == address) return connection; - if (reserve && conn_addr == 0) { - connection->send_service_ = INIT_SENDING_SERVICES; - connection->set_address(address); - // All connections must start at INIT - // We only set the state if we allocate the connection - // to avoid a race where multiple connection attempts - // are made. - connection->set_state(ClientState::INIT); - return connection; - } + if (free_slot == nullptr && conn_addr == 0) + free_slot = connection; } - return nullptr; + if (!reserve || free_slot == nullptr) + return nullptr; + free_slot->send_service_ = INIT_SENDING_SERVICES; + free_slot->set_address(address); + // All connections must start at INIT + // We only set the state if we allocate the connection + // to avoid a race where multiple connection attempts + // are made. + free_slot->set_state(ClientState::INIT); + return free_slot; } void BluetoothProxy::bluetooth_device_request(const api::BluetoothDeviceRequest &msg) { diff --git a/esphome/components/bluetooth_proxy/bluetooth_proxy.h b/esphome/components/bluetooth_proxy/bluetooth_proxy.h index 26f99fcca2..d3e3144831 100644 --- a/esphome/components/bluetooth_proxy/bluetooth_proxy.h +++ b/esphome/components/bluetooth_proxy/bluetooth_proxy.h @@ -242,13 +242,13 @@ class BluetoothProxy final : public Component { std::array connections_{}; #endif ble_device_base::BLEHub *hub_{nullptr}; + // Group 3: 4-byte types; paired with hub_ so the 8-aligned messages below + // start on an even word, closing two alignment holes. + uint32_t last_advertisement_flush_time_{0}; // BLE advertisement batching api::BluetoothLERawAdvertisementsResponse response_; - // Group 3: 4-byte types - uint32_t last_advertisement_flush_time_{0}; - // Pre-allocated response message - always ready to send api::BluetoothConnectionsFreeResponse connections_free_response_; diff --git a/esphome/components/esp32_ble_tracker/__init__.py b/esphome/components/esp32_ble_tracker/__init__.py index 84f43fb54b..634b8c3bef 100644 --- a/esphome/components/esp32_ble_tracker/__init__.py +++ b/esphome/components/esp32_ble_tracker/__init__.py @@ -128,9 +128,7 @@ def validate_max_connections_deprecated(config: ConfigType) -> ConfigType: # 320 ms is the ESP-IDF reference scan interval; the shared schema also # tightens validation to the controller's 2.5 ms .. 10240 ms range and rejects # window/interval pairs that collapse to the same 0.625 ms unit count. -SCAN_PARAMETERS_SCHEMA = ble_device_base.scan_parameters_schema( - "320ms", supports_active=True -) +SCAN_PARAMETERS_SCHEMA = ble_device_base.scan_parameters_schema("320ms") # Codegen helpers are owned by ble_device_base; kept under the historical names # here for the components that import them from this module. diff --git a/esphome/components/ethernet/__init__.py b/esphome/components/ethernet/__init__.py index d1d5e45c6b..8bdd536ffb 100644 --- a/esphome/components/ethernet/__init__.py +++ b/esphome/components/ethernet/__init__.py @@ -132,6 +132,7 @@ ETHERNET_TYPES = { "W6300": EthernetType.ETHERNET_TYPE_W6300, "GENERIC": EthernetType.ETHERNET_TYPE_GENERIC, "YT8531": EthernetType.ETHERNET_TYPE_YT8531, + "CH390": EthernetType.ETHERNET_TYPE_CH390, } # PHY types that need compile-time defines for conditional compilation @@ -153,6 +154,7 @@ _PHY_TYPE_TO_DEFINE = { "W6300": "USE_ETHERNET_W6300", "GENERIC": "USE_ETHERNET_GENERIC", "YT8531": "USE_ETHERNET_YT8531", + "CH390": "USE_ETHERNET_CH390", } @@ -176,13 +178,14 @@ _IDF6_ETHERNET_COMPONENTS: dict[str, IDFRegistryComponent] = { "DM9051": IDFRegistryComponent("espressif/dm9051", "1.1.0"), "ENC28J60": IDFRegistryComponent("espressif/enc28j60", "1.0.1"), "LAN8670": IDFRegistryComponent("espressif/lan867x", "2.0.0"), + "CH390": IDFRegistryComponent("espressif/ch390", "0.3.0"), } # These types are always external IDF components (never built-in to ESP-IDF) -_ALWAYS_EXTERNAL_IDF_COMPONENTS = {"LAN8670", "ENC28J60"} +_ALWAYS_EXTERNAL_IDF_COMPONENTS = {"LAN8670", "ENC28J60", "CH390"} # ESP32-only SPI ethernet types (W5100 is RP2040-only, no ESP-IDF driver) -SPI_ETHERNET_TYPES = {"W5500", "DM9051", "ENC28J60"} +SPI_ETHERNET_TYPES = {"W5500", "DM9051", "ENC28J60", "CH390"} # RP2-supported ethernet types (SPI and PIO QSPI). Applies to the whole # RP2 family (RP2040 and RP2350); the chip-specific W5100 caveat in the # comment above is about ESP-IDF driver coverage, not the RP2 platform. @@ -480,6 +483,12 @@ SPI_SCHEMA = _spi_schema() # of spec for it and makes the driver's CS hold time helper compute no hold SPI_SCHEMA_ENC28J60 = _spi_schema(default_clock="20MHz", max_clock=int(20e6)) +# The CH390H/D rates SCK at 50 MHz typical and 72 MHz maximum with VDDIO at 3.3V, +# so the shared 80 MHz ceiling is out of spec while the 26.67 MHz default is not. +# CH390 datasheet v1.8, tables 9-4 and 9-5: +# https://www.wch-ic.com/downloads/CH390DS1_PDF.html +SPI_SCHEMA_CH390 = _spi_schema(max_clock=int(72e6)) + CONFIG_SCHEMA = cv.All( cv.typed_schema( { @@ -494,6 +503,7 @@ CONFIG_SCHEMA = cv.All( "W5500": SPI_SCHEMA, "OPENETH": cv.All(BASE_SCHEMA, cv.only_on([Platform.ESP32])), "DM9051": SPI_SCHEMA, + "CH390": SPI_SCHEMA_CH390, "ENC28J60": SPI_SCHEMA_ENC28J60, "W6100": cv.All(SPI_SCHEMA, cv.only_on([Platform.RP2])), "W6300": cv.All(SPI_SCHEMA, cv.only_on([Platform.RP2])), @@ -629,8 +639,11 @@ async def _to_code_esp32(var: cg.Pvariable, config: ConfigType) -> None: cg.add(var.set_interface(SPI_INTERFACE_MAP[config[CONF_INTERFACE]])) add_idf_sdkconfig_option("CONFIG_ETH_USE_SPI_ETHERNET", True) # CONFIG_ETH_SPI_ETHERNET_{TYPE} Kconfig options were removed in IDF 6.0 - # ENC28J60 was never built-in to IDF, so it has no Kconfig option - if idf_version() < cv.Version(6, 0, 0) and config[CONF_TYPE] != "ENC28J60": + # Types that are never built into IDF ship no Kconfig option at all + if ( + idf_version() < cv.Version(6, 0, 0) + and config[CONF_TYPE] not in _ALWAYS_EXTERNAL_IDF_COMPONENTS + ): add_idf_sdkconfig_option( f"CONFIG_ETH_SPI_ETHERNET_{config[CONF_TYPE]}", True ) diff --git a/esphome/components/ethernet/ethernet_component.h b/esphome/components/ethernet/ethernet_component.h index 9f4398c621..dc084796e7 100644 --- a/esphome/components/ethernet/ethernet_component.h +++ b/esphome/components/ethernet/ethernet_component.h @@ -88,6 +88,7 @@ enum EthernetType : uint8_t { ETHERNET_TYPE_W6300, ETHERNET_TYPE_GENERIC, ETHERNET_TYPE_YT8531, + ETHERNET_TYPE_CH390, }; struct ManualIP { diff --git a/esphome/components/ethernet/ethernet_component_esp32.cpp b/esphome/components/ethernet/ethernet_component_esp32.cpp index 94f4c23479..7cf8cdf736 100644 --- a/esphome/components/ethernet/ethernet_component_esp32.cpp +++ b/esphome/components/ethernet/ethernet_component_esp32.cpp @@ -50,6 +50,12 @@ #include "esp_eth_enc28j60.h" #endif +// CH390 headers exist on all IDF versions (always an external component) +#ifdef USE_ETHERNET_CH390 +#include "esp_eth_mac_ch390.h" +#include "esp_eth_phy_ch390.h" +#endif + #ifdef USE_ETHERNET_SPI #include #include @@ -215,6 +221,8 @@ void EthernetComponent::ethernet_lazy_init_() { eth_dm9051_config_t dm9051_config = ETH_DM9051_DEFAULT_CONFIG(host, &devcfg); #elif defined(USE_ETHERNET_ENC28J60) eth_enc28j60_config_t enc28j60_config = ETH_ENC28J60_DEFAULT_CONFIG(host, &devcfg); +#elif defined(USE_ETHERNET_CH390) + eth_ch390_config_t ch390_config = ETH_CH390_DEFAULT_CONFIG(host, &devcfg); #endif #if defined(USE_ETHERNET_W5500) @@ -236,6 +244,11 @@ void EthernetComponent::ethernet_lazy_init_() { // time (t10, 210 ns) after the last clock or MAC/MII register reads fail ("wrong chip ID") enc28j60_config.spi_devcfg->cs_ena_posttrans = enc28j60_cal_spi_cs_hold_time((this->clock_speed_ + 999999) / 1000000); enc28j60_config.int_gpio_num = this->interrupt_pin_; +#elif defined(USE_ETHERNET_CH390) + ch390_config.int_gpio_num = this->interrupt_pin_; +#ifdef USE_ETHERNET_SPI_POLLING_SUPPORT + ch390_config.poll_period_ms = this->polling_interval_; +#endif #endif phy_config.phy_addr = this->phy_addr_spi_; @@ -360,6 +373,12 @@ void EthernetComponent::ethernet_lazy_init_() { this->phy_ = esp_eth_phy_new_enc28j60(&phy_config); break; } +#elif defined(USE_ETHERNET_CH390) + case ETHERNET_TYPE_CH390: { + mac = esp_eth_mac_new_ch390(&ch390_config, &mac_config); + this->phy_ = esp_eth_phy_new_ch390(&phy_config); + break; + } #endif #endif default: { @@ -519,6 +538,10 @@ void EthernetComponent::dump_config() { case ETHERNET_TYPE_ENC28J60: eth_type = "ENC28J60"; break; +#elif defined(USE_ETHERNET_CH390) + case ETHERNET_TYPE_CH390: + eth_type = "CH390"; + break; #endif #ifdef USE_ETHERNET_OPENETH case ETHERNET_TYPE_OPENETH: diff --git a/esphome/components/ln882h_ble_tracker/__init__.py b/esphome/components/ln882h_ble_tracker/__init__.py index 8443799144..4bfaa93ab7 100644 --- a/esphome/components/ln882h_ble_tracker/__init__.py +++ b/esphome/components/ln882h_ble_tracker/__init__.py @@ -47,7 +47,7 @@ BLEEndOfScanTrigger = ble_automation.BLEEndOfScanTrigger # LN882H SDK reference scan rate: 100 ms interval / 50 ms window (50 % duty). SCAN_PARAMETERS_SCHEMA = ble_device_base.scan_parameters_schema( - "100ms", window_default="50ms", supports_active=True + "100ms", window_default="50ms" ) diff --git a/esphome/components/mitsubishi_cn105/__init__.py b/esphome/components/mitsubishi_cn105/__init__.py index e69de29bb2..7d5594495a 100644 --- a/esphome/components/mitsubishi_cn105/__init__.py +++ b/esphome/components/mitsubishi_cn105/__init__.py @@ -0,0 +1,137 @@ +from esphome import automation +import esphome.codegen as cg +from esphome.components import uart +import esphome.config_validation as cv +from esphome.const import CONF_ID, CONF_TEMPERATURE, CONF_UPDATE_INTERVAL +from esphome.core import ID +from esphome.cpp_generator import MockObj +from esphome.types import ConfigType, TemplateArgsType + +CODEOWNERS = ["@crnjan"] +DEPENDENCIES = ["uart"] +DOMAIN = "mitsubishi_cn105" + +CONF_MITSUBISHI_CN105_ID = f"{DOMAIN}_id" +CONF_TELEMETRY_REQUEST_MIN_INTERVAL = "telemetry_request_min_interval" + +mitsubishi_ns = cg.esphome_ns.namespace(DOMAIN) + +MitsubishiCN105Component = mitsubishi_ns.class_( + "MitsubishiCN105Component", + cg.Component, + uart.UARTDevice, +) + +SetRemoteTemperatureAction = mitsubishi_ns.class_( + "SetRemoteTemperatureAction", + automation.Action, + cg.Parented.template(MitsubishiCN105Component), +) + +ClearRemoteTemperatureAction = mitsubishi_ns.class_( + "ClearRemoteTemperatureAction", + automation.Action, + cg.Parented.template(MitsubishiCN105Component), +) + +CONFIG_SCHEMA = ( + cv.Schema( + { + cv.GenerateID(): cv.declare_id(MitsubishiCN105Component), + cv.Optional(CONF_UPDATE_INTERVAL, default="1s"): cv.update_interval, + cv.Optional( + CONF_TELEMETRY_REQUEST_MIN_INTERVAL, default="60s" + ): cv.update_interval, + } + ) + .extend(cv.COMPONENT_SCHEMA) + .extend(uart.UART_DEVICE_SCHEMA) +) + +MITSUBISHI_CN105_DEVICE_SCHEMA = cv.Schema( + { + cv.GenerateID(CONF_MITSUBISHI_CN105_ID): cv.use_id(MitsubishiCN105Component), + } +) + +FINAL_VALIDATE_SCHEMA = cv.All( + uart.final_validate_device_schema( + DOMAIN, + require_rx=True, + require_tx=True, + data_bits=8, + parity="EVEN", + stop_bits=1, + ) +) + + +async def register_mitsubishi_cn105_device(var: MockObj, config: ConfigType) -> None: + parent = await cg.get_variable(config[CONF_MITSUBISHI_CN105_ID]) + cg.add(var.set_parent(parent)) + + +async def to_code(config: ConfigType) -> None: + var = cg.new_Pvariable(config[CONF_ID]) + await cg.register_component(var, config) + await uart.register_uart_device(var, config) + cg.add( + var.set_telemetry_request_min_interval( + config[CONF_TELEMETRY_REQUEST_MIN_INTERVAL] + ) + ) + + +REMOTE_TEMPERATURE_ACTION_SCHEMA = cv.Schema( + { + cv.Required(CONF_ID): cv.use_id(MitsubishiCN105Component), + cv.Required(CONF_TEMPERATURE): cv.templatable( + cv.All( + cv.temperature, + cv.Range(min=8.0, max=39.5), + ) + ), + } +) + +CLEAR_REMOTE_TEMPERATURE_ACTION_SCHEMA = cv.Schema( + { + cv.Required(CONF_ID): cv.use_id(MitsubishiCN105Component), + } +) + + +@automation.register_action( + f"{DOMAIN}.set_remote_temperature", + SetRemoteTemperatureAction, + REMOTE_TEMPERATURE_ACTION_SCHEMA, + synchronous=True, +) +async def remote_temperature_action_to_code( + config: ConfigType, + action_id: ID, + template_arg: cg.TemplateArguments, + args: TemplateArgsType, +) -> MockObj: + var = cg.new_Pvariable(action_id, template_arg) + await cg.register_parented(var, config[CONF_ID]) + temperature = await cg.templatable(config[CONF_TEMPERATURE], args, float) + cg.add(var.set_temperature(temperature)) + return var + + +@automation.register_action( + f"{DOMAIN}.clear_remote_temperature", + ClearRemoteTemperatureAction, + CLEAR_REMOTE_TEMPERATURE_ACTION_SCHEMA, + synchronous=True, +) +async def clear_temperature_action_to_code( + config: ConfigType, + action_id: ID, + template_arg: cg.TemplateArguments, + args: TemplateArgsType, +) -> MockObj: + var = cg.new_Pvariable(action_id, template_arg) + await cg.register_parented(var, config[CONF_ID]) + return var diff --git a/esphome/components/mitsubishi_cn105/automation.h b/esphome/components/mitsubishi_cn105/automation.h new file mode 100644 index 0000000000..879e556f9c --- /dev/null +++ b/esphome/components/mitsubishi_cn105/automation.h @@ -0,0 +1,23 @@ +#pragma once + +#include "mitsubishi_cn105_component.h" + +#include "esphome/core/automation.h" + +namespace esphome::mitsubishi_cn105 { + +template +class SetRemoteTemperatureAction : public Action, public Parented { + public: + TEMPLATABLE_VALUE(float, temperature) + + void play(const Ts &...x) override { this->parent_->set_remote_temperature(this->temperature_.value(x...)); } +}; + +template +class ClearRemoteTemperatureAction : public Action, public Parented { + public: + void play(const Ts &...x) override { this->parent_->clear_remote_temperature(); } +}; + +} // namespace esphome::mitsubishi_cn105 diff --git a/esphome/components/mitsubishi_cn105/climate.py b/esphome/components/mitsubishi_cn105/climate.py index 522b9218fc..64475d0e32 100644 --- a/esphome/components/mitsubishi_cn105/climate.py +++ b/esphome/components/mitsubishi_cn105/climate.py @@ -1,3 +1,5 @@ +import logging + from esphome import automation import esphome.codegen as cg from esphome.components import climate, uart @@ -7,126 +9,248 @@ from esphome.const import ( CONF_ID, CONF_SUPPORTED_SWING_MODES, CONF_TEMPERATURE, + CONF_UART_ID, CONF_UPDATE_INTERVAL, ) -from esphome.core import ID +from esphome.core import CORE, ID from esphome.cpp_generator import MockObj +from esphome.schema_extractors import SCHEMA_EXTRACT, schema_extractor from esphome.types import ConfigType, TemplateArgsType +from . import ( + CONF_MITSUBISHI_CN105_ID, + DOMAIN, + MITSUBISHI_CN105_DEVICE_SCHEMA, + MitsubishiCN105Component, + mitsubishi_ns, + register_mitsubishi_cn105_device, +) + +# Legacy climate-owned hub compatibility. Remove in 2027.2.0. DEPENDENCIES = ["uart"] AUTO_LOAD = ["climate"] -CODEOWNERS = ["@crnjan"] +_LOGGER = logging.getLogger(__name__) + +# Deprecated legacy climate-owned hub option. Remove in 2027.2.0. CONF_CURRENT_TEMPERATURE_MIN_INTERVAL = "current_temperature_min_interval" - -mitsubishi_ns = cg.esphome_ns.namespace("mitsubishi_cn105") +# Legacy climate-owned hub compatibility. Remove in 2027.2.0. +CONF_LEGACY_MITSUBISHI_CN105_ID = "legacy_mitsubishi_cn105_id" MitsubishiCN105Climate = mitsubishi_ns.class_( "MitsubishiCN105Climate", climate.Climate, cg.Component, - uart.UARTDevice, + cg.Parented.template(MitsubishiCN105Component), ) -SetRemoteTemperatureAction = mitsubishi_ns.class_( - "SetRemoteTemperatureAction", +# Legacy climate action compatibility. Remove in 2027.2.0. +LegacySetRemoteTemperatureAction = mitsubishi_ns.class_( + "LegacySetRemoteTemperatureAction", automation.Action, cg.Parented.template(MitsubishiCN105Climate), ) -ClearRemoteTemperatureAction = mitsubishi_ns.class_( - "ClearRemoteTemperatureAction", +# Legacy climate action compatibility. Remove in 2027.2.0. +LegacyClearRemoteTemperatureAction = mitsubishi_ns.class_( + "LegacyClearRemoteTemperatureAction", automation.Action, cg.Parented.template(MitsubishiCN105Climate), ) -CONFIG_SCHEMA = ( - climate.climate_schema(MitsubishiCN105Climate) - .extend(uart.UART_DEVICE_SCHEMA) + +# Legacy climate-owned hub compatibility. Remove in 2027.2.0. +def _has_top_level_hub_config() -> bool: + return DOMAIN in (CORE.raw_config or {}) + + +# Legacy climate-owned hub compatibility. Remove in 2027.2.0. +def _prepare_legacy_hub_config(config: ConfigType) -> ConfigType: + _LOGGER.warning( + "Defining 'climate.mitsubishi_cn105' without a top-level '%s:' hub is " + "deprecated. Declare '%s:' and reference it with '%s:' instead. Will " + "be removed in ESPHome 2027.2.0.", + DOMAIN, + DOMAIN, + CONF_MITSUBISHI_CN105_ID, + ) + + # Add the hidden hub declaration only for legacy climate-owned configs, + # so normal auto-ID resolution does not see it as a top-level hub. + config[CONF_LEGACY_MITSUBISHI_CN105_ID] = cv.declare_id(MitsubishiCN105Component)( + None + ) + return config + + +_BASE_SCHEMA = climate.climate_schema(MitsubishiCN105Climate).extend( + { + cv.Optional( + CONF_SUPPORTED_SWING_MODES, default="OFF" + ): validate_climate_swing_mode, + } +) + +_HUB_SCHEMA = _BASE_SCHEMA.extend(MITSUBISHI_CN105_DEVICE_SCHEMA) + +# Hub options accepted in the legacy climate-owned configuration. When a +# top-level hub exists, leaving these on the climate is always a migration +# mistake and the generic schema error does not explain where they belong. +# Legacy climate-owned hub compatibility. Remove in 2027.2.0. +_LEGACY_HUB_KEYS = ( + CONF_CURRENT_TEMPERATURE_MIN_INTERVAL, + CONF_UART_ID, + CONF_UPDATE_INTERVAL, +) + + +# Legacy climate-owned hub compatibility. Remove in 2027.2.0. +def _validate_no_legacy_hub_keys(config: ConfigType) -> ConfigType: + legacy_keys = [key for key in _LEGACY_HUB_KEYS if key in config] + if not legacy_keys: + return config + + keys = ", ".join(f"'{key}'" for key in legacy_keys) + message = f"{keys} must be moved under the top-level '{DOMAIN}:' block" + if CONF_CURRENT_TEMPERATURE_MIN_INTERVAL in legacy_keys: + message += ( + f"; rename '{CONF_CURRENT_TEMPERATURE_MIN_INTERVAL}' to " + "'telemetry_request_min_interval' there" + ) + raise cv.Invalid(message) + + +# Legacy climate-owned hub compatibility. Remove in 2027.2.0. +_LEGACY_SCHEMA = ( + _BASE_SCHEMA.extend(uart.UART_DEVICE_SCHEMA) .extend( { - cv.Optional(CONF_UPDATE_INTERVAL, default="1s"): cv.update_interval, - cv.Optional( - CONF_CURRENT_TEMPERATURE_MIN_INTERVAL, default="60s" - ): cv.update_interval, - cv.Optional( - CONF_SUPPORTED_SWING_MODES, default="OFF" - ): validate_climate_swing_mode, + cv.Optional(CONF_CURRENT_TEMPERATURE_MIN_INTERVAL): cv.update_interval, + cv.Optional(CONF_UPDATE_INTERVAL): cv.update_interval, } ) + .add_extra(_prepare_legacy_hub_config) ) -FINAL_VALIDATE_SCHEMA = cv.All( - uart.final_validate_device_schema( - "mitsubishi_cn105", + +@schema_extractor("schema") +def CONFIG_SCHEMA(config: ConfigType) -> ConfigType: + if config is SCHEMA_EXTRACT: + return _HUB_SCHEMA + if CONF_MITSUBISHI_CN105_ID in config or _has_top_level_hub_config(): + return _HUB_SCHEMA(_validate_no_legacy_hub_keys(config)) + return _LEGACY_SCHEMA(config) + + +# Legacy climate-owned hub compatibility. Remove in 2027.2.0. +def _legacy_final_validate(config: ConfigType) -> ConfigType: + if CONF_MITSUBISHI_CN105_ID in config: + return config + + return uart.final_validate_device_schema( + DOMAIN, require_rx=True, require_tx=True, data_bits=8, parity="EVEN", stop_bits=1, - ) -) + )(config) + + +FINAL_VALIDATE_SCHEMA = _legacy_final_validate async def to_code(config: ConfigType) -> None: var = await climate.new_climate(config) - await cg.register_component(var, config) - await uart.register_uart_device(var, config) - cg.add(var.set_supported_swing_mode(config[CONF_SUPPORTED_SWING_MODES])) - cg.add( - var.set_current_temperature_min_interval( - config[CONF_CURRENT_TEMPERATURE_MIN_INTERVAL] - ) - ) - - -@automation.register_action( - "climate.mitsubishi_cn105.set_remote_temperature", - SetRemoteTemperatureAction, - cv.Schema( - { - cv.Required(CONF_ID): cv.use_id(MitsubishiCN105Climate), - cv.Required(CONF_TEMPERATURE): cv.templatable( - cv.All( - cv.temperature, - cv.Range(min=8.0, max=39.5), + climate_config = config.copy() + # update_interval configures the protocol hub, not the climate entity. + climate_config.pop(CONF_UPDATE_INTERVAL, None) + await cg.register_component(var, climate_config) + if CONF_MITSUBISHI_CN105_ID in config: + await register_mitsubishi_cn105_device(var, config) + else: + # Legacy climate-owned hub compatibility. Remove in 2027.2.0. + parent = cg.new_Pvariable(config[CONF_LEGACY_MITSUBISHI_CN105_ID]) + await cg.register_component(parent, config) + await uart.register_uart_device(parent, config) + if CONF_CURRENT_TEMPERATURE_MIN_INTERVAL in config: + cg.add( + parent.set_telemetry_request_min_interval( + config[CONF_CURRENT_TEMPERATURE_MIN_INTERVAL] ) - ), - } - ), + ) + cg.add(var.set_parent(parent)) + cg.add(var.set_supported_swing_mode(config[CONF_SUPPORTED_SWING_MODES])) + + +# Legacy climate action compatibility. Remove in 2027.2.0. +LEGACY_REMOTE_TEMPERATURE_ACTION_SCHEMA = cv.Schema( + { + cv.Required(CONF_ID): cv.use_id(MitsubishiCN105Climate), + cv.Required(CONF_TEMPERATURE): cv.templatable( + cv.All( + cv.temperature, + cv.Range(min=8.0, max=39.5), + ) + ), + } +) + +# Legacy climate action compatibility. Remove in 2027.2.0. +LEGACY_CLEAR_REMOTE_TEMPERATURE_ACTION_SCHEMA = cv.Schema( + { + cv.Required(CONF_ID): cv.use_id(MitsubishiCN105Climate), + } +) + + +# Legacy climate action compatibility. Remove in 2027.2.0. +@automation.register_action( + f"climate.{DOMAIN}.set_remote_temperature", + LegacySetRemoteTemperatureAction, + LEGACY_REMOTE_TEMPERATURE_ACTION_SCHEMA, synchronous=True, ) -async def set_remote_temperature_action_to_code( +async def legacy_remote_temperature_action_to_code( config: ConfigType, action_id: ID, template_arg: cg.TemplateArguments, args: TemplateArgsType, ) -> MockObj: + _LOGGER.warning( + "The 'climate.%s.set_remote_temperature' action is deprecated. Use " + "'%s.set_remote_temperature' instead. It will be removed in ESPHome " + "2027.2.0.", + DOMAIN, + DOMAIN, + ) var = cg.new_Pvariable(action_id, template_arg) await cg.register_parented(var, config[CONF_ID]) - temperature = await cg.templatable(config[CONF_TEMPERATURE], args, float) cg.add(var.set_temperature(temperature)) - return var +# Legacy climate action compatibility. Remove in 2027.2.0. @automation.register_action( - "climate.mitsubishi_cn105.clear_remote_temperature", - ClearRemoteTemperatureAction, - cv.Schema( - { - cv.Required(CONF_ID): cv.use_id(MitsubishiCN105Climate), - } - ), + f"climate.{DOMAIN}.clear_remote_temperature", + LegacyClearRemoteTemperatureAction, + LEGACY_CLEAR_REMOTE_TEMPERATURE_ACTION_SCHEMA, synchronous=True, ) -async def clear_remote_temperature_action_to_code( +async def legacy_clear_temperature_action_to_code( config: ConfigType, action_id: ID, template_arg: cg.TemplateArguments, args: TemplateArgsType, ) -> MockObj: + _LOGGER.warning( + "The 'climate.%s.clear_remote_temperature' action is deprecated. Use " + "'%s.clear_remote_temperature' instead. It will be removed in ESPHome " + "2027.2.0.", + DOMAIN, + DOMAIN, + ) var = cg.new_Pvariable(action_id, template_arg) await cg.register_parented(var, config[CONF_ID]) return var diff --git a/esphome/components/mitsubishi_cn105/mitsubishi_cn105.cpp b/esphome/components/mitsubishi_cn105/mitsubishi_cn105.cpp index 4782a2ef93..415de34166 100644 --- a/esphome/components/mitsubishi_cn105/mitsubishi_cn105.cpp +++ b/esphome/components/mitsubishi_cn105/mitsubishi_cn105.cpp @@ -1,8 +1,9 @@ +#include "mitsubishi_cn105.h" + #include #include #include #include -#include "mitsubishi_cn105.h" namespace esphome::mitsubishi_cn105 { @@ -25,7 +26,7 @@ static constexpr std::array CONNECT_REQUEST_PAYLOAD = {0xCA, 0x01}; static constexpr uint8_t PACKET_TYPE_STATUS_REQUEST = 0x42; static constexpr uint8_t PACKET_TYPE_STATUS_RESPONSE = 0x62; static constexpr uint8_t STATUS_MSG_SETTINGS = 0x02; -static constexpr uint8_t STATUS_MSG_ROOM_TEMP = 0x03; +static constexpr uint8_t STATUS_MSG_TELEMETRY = 0x03; static constexpr uint8_t PACKET_TYPE_WRITE_SETTINGS_REQUEST = 0x41; static constexpr uint8_t PACKET_TYPE_WRITE_SETTINGS_RESPONSE = 0x61; @@ -229,8 +230,8 @@ void MitsubishiCN105::did_transition_(State to) { case State::STATUS_UPDATED: { if (this->pending_updates_.any() && this->is_status_initialized()) { this->set_state_(State::APPLYING_SETTINGS); - } else if (this->current_status_msg_type_ == STATUS_MSG_SETTINGS && this->should_request_room_temperature_()) { - this->current_status_msg_type_ = STATUS_MSG_ROOM_TEMP; + } else if (this->current_status_msg_type_ == STATUS_MSG_SETTINGS && this->should_request_telemetry_()) { + this->current_status_msg_type_ = STATUS_MSG_TELEMETRY; this->set_state_(State::UPDATING_STATUS); } else { this->set_state_(State::SCHEDULE_NEXT_STATUS_UPDATE); @@ -264,16 +265,16 @@ void MitsubishiCN105::did_transition_(State to) { } } -bool MitsubishiCN105::should_request_room_temperature_() const { - if (!this->is_room_temperature_enabled()) { +bool MitsubishiCN105::should_request_telemetry_() const { + if (!this->is_telemetry_polling_enabled()) { return false; } - if (!this->last_room_temperature_update_ms_.has_value()) { + if (!this->last_telemetry_update_ms_.has_value()) { return true; } - return (get_loop_time_ms() - *this->last_room_temperature_update_ms_) >= this->room_temperature_min_interval_ms_; + return (get_loop_time_ms() - *this->last_telemetry_update_ms_) >= this->telemetry_request_min_interval_ms_; } void MitsubishiCN105::send_packet_(const uint8_t *packet, size_t len) { @@ -327,7 +328,7 @@ bool MitsubishiCN105::process_status_packet_(const uint8_t *payload, size_t len) previous.fan_mode != this->status_.fan_mode || previous.target_temperature != this->status_.target_temperature || previous.vane_mode != this->status_.vane_mode || previous.wide_vane_mode != this->status_.wide_vane_mode; - if (this->is_room_temperature_enabled()) { + if (this->is_telemetry_polling_enabled()) { changed |= previous.room_temperature != this->status_.room_temperature; } @@ -339,8 +340,8 @@ bool MitsubishiCN105::parse_status_payload_(uint8_t msg_type, const uint8_t *pay case STATUS_MSG_SETTINGS: return this->parse_status_settings_(payload, len); - case STATUS_MSG_ROOM_TEMP: - return this->parse_status_room_temperature_(payload, len); + case STATUS_MSG_TELEMETRY: + return this->parse_status_telemetry_(payload, len); default: ESP_LOGVV(TAG, "RX unsupported status msg type 0x%02X", msg_type); @@ -384,14 +385,14 @@ bool MitsubishiCN105::parse_status_settings_(const uint8_t *payload, size_t len) return true; } -bool MitsubishiCN105::parse_status_room_temperature_(const uint8_t *payload, size_t len) { +bool MitsubishiCN105::parse_status_telemetry_(const uint8_t *payload, size_t len) { if (len <= 5) { - ESP_LOGVV(TAG, "RX room temperature payload too short"); + ESP_LOGVV(TAG, "RX telemetry payload too short"); return false; } this->status_.room_temperature = decode_temperature(payload[2], payload[5], 10); - this->last_room_temperature_update_ms_ = get_loop_time_ms(); + this->last_telemetry_update_ms_ = get_loop_time_ms(); return true; } diff --git a/esphome/components/mitsubishi_cn105/mitsubishi_cn105.h b/esphome/components/mitsubishi_cn105/mitsubishi_cn105.h index 742d8e18a9..3169359290 100644 --- a/esphome/components/mitsubishi_cn105/mitsubishi_cn105.h +++ b/esphome/components/mitsubishi_cn105/mitsubishi_cn105.h @@ -1,9 +1,10 @@ #pragma once +#include "esphome/components/uart/uart.h" +#include "esphome/core/finite_set_mask.h" + #include #include -#include "esphome/components/uart/uart.h" -#include "esphome/core/finite_set_mask.h" namespace esphome::mitsubishi_cn105 { @@ -70,16 +71,16 @@ class MitsubishiCN105 { uint32_t get_update_interval() const { return this->update_interval_ms_; } void set_update_interval(uint32_t interval_ms) { this->update_interval_ms_ = interval_ms; } - uint32_t get_room_temperature_min_interval() const { return this->room_temperature_min_interval_ms_; } - bool is_room_temperature_enabled() const { return this->room_temperature_min_interval_ms_ != SCHEDULER_DONT_RUN; } - void set_room_temperature_min_interval(uint32_t interval_ms) { - this->room_temperature_min_interval_ms_ = interval_ms; + uint32_t get_telemetry_request_min_interval() const { return this->telemetry_request_min_interval_ms_; } + bool is_telemetry_polling_enabled() const { return this->telemetry_request_min_interval_ms_ != SCHEDULER_DONT_RUN; } + void set_telemetry_request_min_interval(uint32_t interval_ms) { + this->telemetry_request_min_interval_ms_ = interval_ms; } const Status &status() const { return this->status_; } bool is_status_initialized() const { - return this->is_room_temperature_enabled() ? !std::isnan(this->status_.room_temperature) - : !std::isnan(this->status_.target_temperature); + return this->is_telemetry_polling_enabled() ? !std::isnan(this->status_.room_temperature) + : !std::isnan(this->status_.target_temperature); } void set_power(bool power_on); @@ -150,10 +151,10 @@ class MitsubishiCN105 { bool process_status_packet_(const uint8_t *payload, size_t len); bool parse_status_payload_(uint8_t msg_type, const uint8_t *payload, size_t len); bool parse_status_settings_(const uint8_t *payload, size_t len); - bool parse_status_room_temperature_(const uint8_t *payload, size_t len); + bool parse_status_telemetry_(const uint8_t *payload, size_t len); void send_packet_(const uint8_t *packet, size_t len); void update_status_(); - bool should_request_room_temperature_() const; + bool should_request_telemetry_() const; void apply_settings_(); bool has_timed_out_(uint32_t timeout) const { return ((get_loop_time_ms() - this->operation_start_ms_) >= timeout); } void set_remote_temperature_half_deg_(uint8_t temperature_half_deg); @@ -162,11 +163,15 @@ class MitsubishiCN105 { static const LogString *state_to_string(State state); uart::UARTDevice &device_; + // Default 1s; legacy climate-owned hub compatibility relies on this when update_interval is omitted. + // Remove legacy note in 2027.2.0. uint32_t update_interval_ms_{1000}; uint32_t status_update_wait_credit_ms_{0}; uint32_t operation_start_ms_{0}; - uint32_t room_temperature_min_interval_ms_{60000}; - std::optional last_room_temperature_update_ms_; + // Default 60s; legacy climate-owned hub compatibility relies on this when current_temperature_min_interval is + // omitted. Remove legacy note in 2027.2.0. + uint32_t telemetry_request_min_interval_ms_{60000}; + std::optional last_telemetry_update_ms_; Status status_{}; State state_{State::NOT_CONNECTED}; UpdateFlags pending_updates_; diff --git a/esphome/components/mitsubishi_cn105/mitsubishi_cn105_climate.cpp b/esphome/components/mitsubishi_cn105/mitsubishi_cn105_climate.cpp index afffe7ea5e..13e02668d1 100644 --- a/esphome/components/mitsubishi_cn105/mitsubishi_cn105_climate.cpp +++ b/esphome/components/mitsubishi_cn105/mitsubishi_cn105_climate.cpp @@ -1,5 +1,5 @@ -#include #include "mitsubishi_cn105_climate.h" + #include "esphome/core/log.h" namespace esphome::mitsubishi_cn105 { @@ -50,25 +50,11 @@ static constexpr std::optional reverse_map_lookup(const std::arrayhp_.is_room_temperature_enabled()) { - ESP_LOGCONFIG(TAG, " Current temperature min interval: %" PRIu32 " ms", - this->hp_.get_room_temperature_min_interval()); - } else { - ESP_LOGCONFIG(TAG, " Current temperature: DISABLED"); - } - ESP_LOGCONFIG(TAG, - " Update interval: %" PRIu32 " ms\n" - " UART: baud_rate=%" PRIu32 " data_bits=%u parity=%s stop_bits=%u", - this->hp_.get_update_interval(), this->parent_->get_baud_rate(), this->parent_->get_data_bits(), - LOG_STR_ARG(parity_to_str(this->parent_->get_parity())), this->parent_->get_stop_bits()); -} +void MitsubishiCN105Climate::dump_config() { LOG_CLIMATE("", "Mitsubishi CN105 Climate", this); } -void MitsubishiCN105Climate::setup() { this->hp_.initialize(); } - -void MitsubishiCN105Climate::loop() { - if (this->hp_.update()) { +void MitsubishiCN105Climate::setup() { + this->parent_->add_on_status_callback([this]() { this->apply_values_(); }); + if (this->parent_->is_status_initialized()) { this->apply_values_(); } } @@ -90,7 +76,7 @@ climate::ClimateTraits MitsubishiCN105Climate::traits() { traits.set_visual_max_temperature(31.0f); traits.set_visual_temperature_step(1.0f); - if (this->hp_.is_room_temperature_enabled()) { + if (this->parent_->is_telemetry_polling_enabled()) { traits.add_feature_flags(climate::CLIMATE_SUPPORTS_CURRENT_TEMPERATURE); traits.set_visual_current_temperature_step(0.5f); } @@ -100,20 +86,20 @@ climate::ClimateTraits MitsubishiCN105Climate::traits() { void MitsubishiCN105Climate::control(const climate::ClimateCall &call) { if (const auto target_temperature = call.get_target_temperature()) { - this->hp_.set_target_temperature(*target_temperature); + this->parent_->set_target_temperature(*target_temperature); } if (const auto mode = call.get_mode()) { if (*mode == climate::CLIMATE_MODE_OFF) { - this->hp_.set_power(false); + this->parent_->set_power(false); } else if (const auto mapped = reverse_map_lookup(MODE_MAP, *mode)) { - this->hp_.set_power(true); - this->hp_.set_mode(*mapped); + this->parent_->set_power(true); + this->parent_->set_mode(*mapped); } } if (const auto fan_mode = reverse_map_lookup(FAN_MODE_MAP, call.get_fan_mode())) { - this->hp_.set_fan_mode(*fan_mode); + this->parent_->set_fan_mode(*fan_mode); } if (const auto swing_mode = call.get_swing_mode()) { @@ -140,24 +126,24 @@ void MitsubishiCN105Climate::control(const climate::ClimateCall &call) { } if (this->supported_swing_modes_.count(climate::CLIMATE_SWING_VERTICAL)) { - this->hp_.set_vane_mode(vane); + this->parent_->set_vane_mode(vane); } if (this->supported_swing_modes_.count(climate::CLIMATE_SWING_HORIZONTAL)) { - this->hp_.set_wide_vane_mode(wide); + this->parent_->set_wide_vane_mode(wide); } } - if (this->hp_.is_status_initialized()) { + if (this->parent_->is_status_initialized()) { this->apply_values_(); } } void MitsubishiCN105Climate::apply_values_() { - const auto &status = this->hp_.status(); + const auto &status = this->parent_->status(); this->target_temperature = status.target_temperature; - if (this->hp_.is_room_temperature_enabled()) { + if (this->parent_->is_telemetry_polling_enabled()) { this->current_temperature = status.room_temperature; } diff --git a/esphome/components/mitsubishi_cn105/mitsubishi_cn105_climate.h b/esphome/components/mitsubishi_cn105/mitsubishi_cn105_climate.h index c83a5519c1..5341c2d2d9 100644 --- a/esphome/components/mitsubishi_cn105/mitsubishi_cn105_climate.h +++ b/esphome/components/mitsubishi_cn105/mitsubishi_cn105_climate.h @@ -1,51 +1,47 @@ #pragma once +#include "mitsubishi_cn105_component.h" +#include "mitsubishi_cn105.h" + #include "esphome/core/automation.h" #include "esphome/core/component.h" #include "esphome/components/climate/climate.h" -#include "esphome/components/uart/uart.h" -#include "mitsubishi_cn105.h" namespace esphome::mitsubishi_cn105 { -class MitsubishiCN105Climate : public climate::Climate, public Component, public uart::UARTDevice { +class MitsubishiCN105Climate : public climate::Climate, public Component, public Parented { public: - explicit MitsubishiCN105Climate() : hp_(*this) {} - void setup() override; - void loop() override; void dump_config() override; climate::ClimateTraits traits() override; void control(const climate::ClimateCall &call) override; - void set_update_interval(uint32_t ms) { this->hp_.set_update_interval(ms); } - void set_current_temperature_min_interval(uint32_t ms) { this->hp_.set_room_temperature_min_interval(ms); } - - void set_remote_temperature(float temperature) { this->hp_.set_remote_temperature(temperature); } - void clear_remote_temperature() { this->hp_.clear_remote_temperature(); } - void set_supported_swing_mode(climate::ClimateSwingMode mode); + // Legacy climate action compatibility. Remove in 2027.2.0. + void set_remote_temperature(float temperature) { this->parent_->set_remote_temperature(temperature); } + void clear_remote_temperature() { this->parent_->clear_remote_temperature(); } protected: void apply_values_(); - MitsubishiCN105 hp_; climate::ClimateSwingModeMask supported_swing_modes_{}; MitsubishiCN105::VaneMode last_non_swing_vane_mode_{MitsubishiCN105::VaneMode::AUTO}; MitsubishiCN105::WideVaneMode last_non_swing_wide_vane_mode_{MitsubishiCN105::WideVaneMode::CENTER}; }; +// Legacy climate action compatibility. Remove in 2027.2.0. template -class SetRemoteTemperatureAction : public Action, public Parented { +class LegacySetRemoteTemperatureAction : public Action, public Parented { public: TEMPLATABLE_VALUE(float, temperature) void play(const Ts &...x) override { this->parent_->set_remote_temperature(this->temperature_.value(x...)); } }; +// Legacy climate action compatibility. Remove in 2027.2.0. template -class ClearRemoteTemperatureAction : public Action, public Parented { +class LegacyClearRemoteTemperatureAction : public Action, public Parented { public: void play(const Ts &...x) override { this->parent_->clear_remote_temperature(); } }; diff --git a/esphome/components/mitsubishi_cn105/mitsubishi_cn105_component.cpp b/esphome/components/mitsubishi_cn105/mitsubishi_cn105_component.cpp new file mode 100644 index 0000000000..166e7fbf88 --- /dev/null +++ b/esphome/components/mitsubishi_cn105/mitsubishi_cn105_component.cpp @@ -0,0 +1,34 @@ +#include "mitsubishi_cn105_component.h" + +#include "esphome/core/log.h" + +#include + +namespace esphome::mitsubishi_cn105 { + +static const char *const TAG = "mitsubishi_cn105"; + +void MitsubishiCN105Component::dump_config() { + ESP_LOGCONFIG(TAG, "Mitsubishi CN105:"); + if (this->hp_.is_telemetry_polling_enabled()) { + ESP_LOGCONFIG(TAG, " Telemetry polling min interval: %" PRIu32 " ms", + this->hp_.get_telemetry_request_min_interval()); + } else { + ESP_LOGCONFIG(TAG, " Telemetry polling: DISABLED"); + } + ESP_LOGCONFIG(TAG, + " Update interval: %" PRIu32 " ms\n" + " UART: baud_rate=%" PRIu32 " data_bits=%u parity=%s stop_bits=%u", + this->hp_.get_update_interval(), this->parent_->get_baud_rate(), this->parent_->get_data_bits(), + LOG_STR_ARG(parity_to_str(this->parent_->get_parity())), this->parent_->get_stop_bits()); +} + +void MitsubishiCN105Component::setup() { this->hp_.initialize(); } + +void MitsubishiCN105Component::loop() { + if (this->hp_.update()) { + this->status_callback_.call(); + } +} + +} // namespace esphome::mitsubishi_cn105 diff --git a/esphome/components/mitsubishi_cn105/mitsubishi_cn105_component.h b/esphome/components/mitsubishi_cn105/mitsubishi_cn105_component.h new file mode 100644 index 0000000000..2319ea7c54 --- /dev/null +++ b/esphome/components/mitsubishi_cn105/mitsubishi_cn105_component.h @@ -0,0 +1,46 @@ +#pragma once + +#include "mitsubishi_cn105.h" + +#include "esphome/core/component.h" +#include "esphome/components/uart/uart.h" + +#include + +namespace esphome::mitsubishi_cn105 { + +class MitsubishiCN105Component : public Component, public uart::UARTDevice { + public: + explicit MitsubishiCN105Component() : hp_(*this) {} + + void setup() override; + void loop() override; + void dump_config() override; + + void set_update_interval(uint32_t ms) { this->hp_.set_update_interval(ms); } + void set_telemetry_request_min_interval(uint32_t ms) { this->hp_.set_telemetry_request_min_interval(ms); } + + void set_remote_temperature(float temperature) { this->hp_.set_remote_temperature(temperature); } + void clear_remote_temperature() { this->hp_.clear_remote_temperature(); } + + void set_power(bool power_on) { this->hp_.set_power(power_on); } + void set_target_temperature(float target_temperature) { this->hp_.set_target_temperature(target_temperature); } + void set_mode(MitsubishiCN105::Mode mode) { this->hp_.set_mode(mode); } + void set_fan_mode(MitsubishiCN105::FanMode fan_mode) { this->hp_.set_fan_mode(fan_mode); } + void set_vane_mode(MitsubishiCN105::VaneMode vane_mode) { this->hp_.set_vane_mode(vane_mode); } + void set_wide_vane_mode(MitsubishiCN105::WideVaneMode mode) { this->hp_.set_wide_vane_mode(mode); } + + const MitsubishiCN105::Status &status() const { return this->hp_.status(); } + bool is_status_initialized() const { return this->hp_.is_status_initialized(); } + bool is_telemetry_polling_enabled() const { return this->hp_.is_telemetry_polling_enabled(); } + + template void add_on_status_callback(F &&callback) { + this->status_callback_.add(std::forward(callback)); + } + + protected: + MitsubishiCN105 hp_; + CallbackManager status_callback_; +}; + +} // namespace esphome::mitsubishi_cn105 diff --git a/esphome/components/modbus/__init__.py b/esphome/components/modbus/__init__.py index 377dadad76..58bd0f65dc 100644 --- a/esphome/components/modbus/__init__.py +++ b/esphome/components/modbus/__init__.py @@ -28,6 +28,7 @@ MAX_NUM_OF_DISCRETE_INPUTS_TO_READ = 2000 MAX_NUM_OF_COILS_TO_WRITE = 1968 MAX_NUM_OF_REGISTERS_TO_READ = 125 MAX_NUM_OF_REGISTERS_TO_WRITE = 123 +MAX_NUM_OF_REGISTERS_TO_WRITE_RW = 121 modbus_ns = cg.esphome_ns.namespace("modbus") Modbus = modbus_ns.class_("Modbus", cg.Component, uart.UARTDevice) diff --git a/esphome/components/modbus/modbus.cpp b/esphome/components/modbus/modbus.cpp index 9f2527d9fb..cff086aeea 100644 --- a/esphome/components/modbus/modbus.cpp +++ b/esphome/components/modbus/modbus.cpp @@ -1,4 +1,7 @@ #include "modbus.h" + +#include + #include "esphome/core/application.h" #include "esphome/core/helpers.h" #include "esphome/core/log.h" @@ -378,10 +381,9 @@ ModbusServerDevice *ModbusServerHub::find_device_(uint8_t address) { return nullptr; } -ResponseStatus ModbusServerHub::check_register_range_(uint16_t start_address, uint16_t number_of_registers) { - if ((uint32_t) start_address + number_of_registers > 0x10000u) { - ESP_LOGW(TAG, "Register address out of range - start: %" PRIu16 " num: %" PRIu16, start_address, - number_of_registers); +ResponseStatus ModbusServerHub::check_address_range_(uint16_t start_address, uint16_t count) { + if (!helpers::address_range_fits(start_address, count)) { + ESP_LOGW(TAG, "Address out of range - start: %" PRIu16 " num: %" PRIu16, start_address, count); return ExceptionCode::ILLEGAL_DATA_ADDRESS; } return std::nullopt; @@ -394,6 +396,11 @@ static constexpr size_t WRITE_SINGLE_VALUES_OFFSET = 2; static constexpr size_t WRITE_MULTIPLE_VALUES_OFFSET = 5; // FC 0x17 writes follow read start(2) + read quantity(2) + write start(2) + write quantity(2) + byte count(1). static constexpr size_t READ_WRITE_VALUES_OFFSET = 9; +// A coil write (FC 0x0F) is function(1) + start(2) + quantity(2) + byte count(1) + packed bits. The largest +// one (MAX_NUM_OF_COILS_TO_WRITE coils) must fit the received request PDU, so the value subspan taken at +// WRITE_MULTIPLE_VALUES_OFFSET can never run past it. +static_assert(1 + WRITE_MULTIPLE_VALUES_OFFSET + packed_bit_bytes(MAX_NUM_OF_COILS_TO_WRITE) <= MAX_PDU_SIZE, + "the largest FC 0x0F coil write must fit within MAX_PDU_SIZE"); ResponseStatus ModbusServerHub::parse_write_single_(std::span data, uint16_t &start_address, RegisterValues ®isters) { @@ -413,13 +420,59 @@ ResponseStatus ModbusServerHub::parse_write_multiple_(std::span d ESP_LOGW(TAG, "Invalid number of registers %" PRIu16 " or bytes %" PRIu8, number_of_registers, number_of_bytes); return ExceptionCode::ILLEGAL_DATA_VALUE; } - if (ResponseStatus status = this->check_register_range_(start_address, number_of_registers); status.has_value()) { + if (ResponseStatus status = this->check_address_range_(start_address, number_of_registers); status.has_value()) { return status; } this->assemble_registers_(data.subspan(WRITE_MULTIPLE_VALUES_OFFSET, number_of_bytes), registers); return std::nullopt; } +ResponseStatus ModbusServerHub::parse_read_request_(std::span data, uint16_t max_entities, + const LogString *entity_name, uint16_t &start_address, + uint16_t &count) { + // Every read request is start address(2) + quantity(2); only the protocol ceiling differs per function + // code, so registers and coils/discrete inputs validate through here and cannot drift apart. + start_address = helpers::get_data(data.data(), 0); + count = helpers::get_data(data.data(), 2); + if (count == 0 || count > max_entities) { + ESP_LOGW(TAG, "Invalid number of %s %" PRIu16, LOG_STR_ARG(entity_name), count); + return ExceptionCode::ILLEGAL_DATA_VALUE; + } + return this->check_address_range_(start_address, count); +} + +ResponseStatus ModbusServerHub::parse_write_single_coil_(std::span data, uint16_t &start_address, + bool &value) { + start_address = helpers::get_data(data.data(), 0); + const uint16_t raw_value = helpers::get_data(data.data(), WRITE_SINGLE_VALUES_OFFSET); + if (raw_value != 0xFF00 && raw_value != 0x0000) { + ESP_LOGW(TAG, "Invalid coil value 0x%04X", raw_value); + return ExceptionCode::ILLEGAL_DATA_VALUE; + } + // No range check needed: one coil can never push start_address + 1 past the address space. + value = raw_value == 0xFF00; + return std::nullopt; +} + +ResponseStatus ModbusServerHub::parse_write_multiple_coils_(std::span data, uint16_t &start_address, + uint16_t &count, std::span &packed_bytes) { + start_address = helpers::get_data(data.data(), 0); + const uint16_t number_of_bits = helpers::get_data(data.data(), 2); + const uint8_t number_of_bytes = helpers::get_data(data.data(), 4); + if (number_of_bits == 0 || number_of_bits > MAX_NUM_OF_COILS_TO_WRITE || + packed_bit_bytes(number_of_bits) != number_of_bytes) { + ESP_LOGW(TAG, "Invalid number of coils %" PRIu16 " or bytes %" PRIu8, number_of_bits, number_of_bytes); + return ExceptionCode::ILLEGAL_DATA_VALUE; + } + if (ResponseStatus status = this->check_address_range_(start_address, number_of_bits); status.has_value()) { + return status; + } + count = number_of_bits; + // coil values follow start(2) + quantity(2) + byte count(1) + packed_bytes = data.subspan(WRITE_MULTIPLE_VALUES_OFFSET, number_of_bytes); + return std::nullopt; +} + void ModbusServerHub::assemble_registers_(std::span values, RegisterValues ®isters) { for (size_t offset = 0; offset + 1 < values.size(); offset += 2) { registers.push_back(helpers::get_data(values.data(), offset)); @@ -427,11 +480,16 @@ void ModbusServerHub::assemble_registers_(std::span values, Regis } void ModbusServerHub::process_broadcast_frame_(uint8_t function_code, std::span data) { - // Broadcasts are only meaningful for register writes and are never answered (Modbus 4.1 / 6.12), so an - // unsupported function code or a validation failure is silently dropped instead of replying with an exception. - // Coil writes (FC 0x05/0x0F) are also broadcastable by spec, but server coil handlers are not implemented yet. + // Broadcasts are only meaningful for writes and are never answered (Modbus 4.1 / 6.12), so an unsupported + // function code or a validation failure is silently dropped instead of replying with an exception. Both + // register writes (FC 0x06/0x10) and coil writes (FC 0x05/0x0F) are broadcastable by spec, and each shares + // its parser with the addressed path so a broadcast is validated exactly as the unicast form would be. uint16_t start_address; RegisterValues registers; + uint16_t coil_count = 0; + std::span packed_bytes; + uint8_t single_bit = 0; // backs packed_bytes for a single-coil write, so it must outlive the loop below + bool coils = false; ResponseStatus status; switch (static_cast(function_code)) { case FunctionCode::WRITE_SINGLE_REGISTER: @@ -440,6 +498,19 @@ void ModbusServerHub::process_broadcast_frame_(uint8_t function_code, std::span< case FunctionCode::WRITE_MULTIPLE_REGISTERS: status = this->parse_write_multiple_(data, start_address, registers); break; + case FunctionCode::WRITE_SINGLE_COIL: { + coils = true; + bool value = false; + status = this->parse_write_single_coil_(data, start_address, value); + single_bit = value ? 0x01 : 0x00; + coil_count = 1; + packed_bytes = std::span(&single_bit, 1); + break; + } + case FunctionCode::WRITE_MULTIPLE_COILS: + coils = true; + status = this->parse_write_multiple_coils_(data, start_address, coil_count, packed_bytes); + break; default: // Reads and read/write require a reply, so they are not valid as broadcasts. ESP_LOGV(TAG, "Ignoring broadcast with unsupported function code %" PRIu8, function_code); @@ -452,8 +523,12 @@ void ModbusServerHub::process_broadcast_frame_(uint8_t function_code, std::span< // per-device outcome at V, and warn if the write reached nobody at all. bool accepted = false; for (auto *device : this->devices_) { - if (ResponseStatus device_status = device->on_broadcast_write_registers(start_address, registers); - device_status.has_value()) { + // Same handlers as an addressed write - a device cannot tell a broadcast apart, and does not need + // to: the hub owns the difference, which is only that no reply is ever sent. + const ResponseStatus device_status = + coils ? device->on_write_coils(start_address, PackedBits(packed_bytes, coil_count)) + : device->on_write_registers(start_address, registers); + if (device_status.has_value()) { ESP_LOGV(TAG, "Device %" PRIu8 " rejected broadcast write with exception %" PRIu8, device->get_address(), static_cast(device_status.value())); } else { @@ -461,15 +536,19 @@ void ModbusServerHub::process_broadcast_frame_(uint8_t function_code, std::span< } } if (!accepted && !this->devices_.empty()) { + const uint16_t entity_count = coils ? coil_count : static_cast(registers.size()); + const LogString *const entity_name = coils ? LOG_STR("coils") : LOG_STR("registers"); // Warn at most once per interval, then drop to VERBOSE: on a shared bus a broadcast aimed at other nodes // repeats forever, so warning per frame would flood the log. const uint32_t now = millis(); if (this->last_unaccepted_broadcast_warn_ == 0 || now - this->last_unaccepted_broadcast_warn_ > UNACCEPTED_BROADCAST_WARN_INTERVAL_MS) { this->last_unaccepted_broadcast_warn_ = now; - ESP_LOGW(TAG, "No device accepted broadcast write of %zu registers at 0x%04X", registers.size(), start_address); + ESP_LOGW(TAG, "No device accepted broadcast write of %" PRIu16 " %s at 0x%04X", entity_count, + LOG_STR_ARG(entity_name), start_address); } else { - ESP_LOGV(TAG, "No device accepted broadcast write of %zu registers at 0x%04X", registers.size(), start_address); + ESP_LOGV(TAG, "No device accepted broadcast write of %" PRIu16 " %s at 0x%04X", entity_count, + LOG_STR_ARG(entity_name), start_address); } } } @@ -479,8 +558,7 @@ bool ModbusServerHub::build_or_reject_read_response_(uint8_t address, uint8_t fu std::span response_buffer, uint16_t &response_len) { // A handler that returns an exception leaves registers partially filled, so check the exception // first and forward it before validating the register count on the success path. - if (status.has_value()) { - this->send_exception_(address, function_code, status.value()); + if (this->rejected_(address, function_code, status)) { return false; } @@ -535,17 +613,11 @@ void ModbusServerHub::process_modbus_client_frame_(uint8_t address, uint8_t func switch (static_cast(function_code)) { case FunctionCode::READ_HOLDING_REGISTERS: case FunctionCode::READ_INPUT_REGISTERS: { - // PDU data: start address(2) + quantity(2). - uint16_t start_address = helpers::get_data(data.data(), 0); - uint16_t number_of_registers = helpers::get_data(data.data(), 2); - if (number_of_registers == 0 || number_of_registers > MAX_NUM_OF_REGISTERS_TO_READ) { - ESP_LOGW(TAG, "Invalid number of registers %" PRIu16, number_of_registers); - this->send_exception_(address, function_code, ExceptionCode::ILLEGAL_DATA_VALUE); - return; - } - status = this->check_register_range_(start_address, number_of_registers); - if (status.has_value()) { - this->send_exception_(address, function_code, status.value()); + uint16_t start_address; + uint16_t number_of_registers; + status = this->parse_read_request_(data, MAX_NUM_OF_REGISTERS_TO_READ, LOG_STR("registers"), start_address, + number_of_registers); + if (this->rejected_(address, function_code, status)) { return; } RegisterValues registers; @@ -571,8 +643,7 @@ void ModbusServerHub::process_modbus_client_frame_(uint8_t address, uint8_t func } else { status = this->parse_write_multiple_(data, start_address, registers); } - if (status.has_value()) { - this->send_exception_(address, function_code, status.value()); + if (this->rejected_(address, function_code, status)) { return; } status = device->on_write_registers(start_address, registers); @@ -580,6 +651,64 @@ void ModbusServerHub::process_modbus_client_frame_(uint8_t address, uint8_t func response_len = 4; break; } + case FunctionCode::READ_COILS: + case FunctionCode::READ_DISCRETE_INPUTS: { + uint16_t start_address; + uint16_t number_of_bits; + status = + this->parse_read_request_(data, MAX_NUM_OF_COILS_TO_READ, LOG_STR("bits"), start_address, number_of_bits); + if (this->rejected_(address, function_code, status)) { + return; + } + // Response: byte count(1) + packed bytes, written straight into the pre-zeroed response buffer. It + // always fits: the parse above caps the count, and a static_assert bounds that against MAX_RAW_SIZE. + const uint8_t byte_count = static_cast(packed_bit_bytes(number_of_bits)); + response_buffer[response_len++] = byte_count; + // Take the packed-bytes span off a span that knows response_buffer's real size, so a future non-zero + // response_len (e.g. a prefix written before the packed data) is a bounds error, not a silent overrun. + std::span packed_out = std::span(response_buffer).subspan(response_len, byte_count); + std::fill(packed_out.begin(), packed_out.end(), 0); + MutablePackedBits bits(packed_out, number_of_bits); + if (static_cast(function_code) == FunctionCode::READ_COILS) { + status = device->on_read_coils(start_address, bits); + } else { + status = device->on_read_discrete_inputs(start_address, bits); + } + if (this->rejected_(address, function_code, status)) { + return; + } + response_len += byte_count; + break; + } + case FunctionCode::WRITE_SINGLE_COIL: { + // A single coil is handed to the device as a one-bit packed view, the same form a multiple-coil + // write takes, so a device only ever implements one coil write handler. + uint16_t start_address; + bool value = false; + status = this->parse_write_single_coil_(data, start_address, value); + if (this->rejected_(address, function_code, status)) { + return; + } + const uint8_t single_bit = value ? 0x01 : 0x00; + status = device->on_write_coils(start_address, PackedBits(std::span(&single_bit, 1), 1)); + response_data = data.data(); // echo the request header per Modbus 6.5, 6.11 + response_len = 4; + break; + } + case FunctionCode::WRITE_MULTIPLE_COILS: { + // Parse and validate the coil write PDU into a packed-bit view; reply with an exception on failure. + uint16_t start_address; + uint16_t count; + std::span packed_bytes; + status = this->parse_write_multiple_coils_(data, start_address, count, packed_bytes); + if (this->rejected_(address, function_code, status)) { + return; + } + status = device->on_write_coils(start_address, PackedBits(packed_bytes, count)); + response_data = data.data(); // echo the request header per Modbus 6.5, 6.11 + response_len = 4; + break; + } case FunctionCode::READ_WRITE_MULTIPLE_REGISTERS: { // PDU data: read start address(2) + read quantity(2) + write start address(2) + write quantity(2) + // write byte count(1) + write register values. Per Modbus 6.17 the write is performed before the read. @@ -596,12 +725,11 @@ void ModbusServerHub::process_modbus_client_frame_(uint8_t address, uint8_t func this->send_exception_(address, function_code, ExceptionCode::ILLEGAL_DATA_VALUE); return; } - status = this->check_register_range_(read_start_address, number_of_registers); + status = this->check_address_range_(read_start_address, number_of_registers); if (!status.has_value()) { - status = this->check_register_range_(write_start_address, number_of_write_registers); + status = this->check_address_range_(write_start_address, number_of_write_registers); } - if (status.has_value()) { - this->send_exception_(address, function_code, status.value()); + if (this->rejected_(address, function_code, status)) { return; } // Perform the write first (Modbus 6.17). Scoped so the write values are off the stack before the read @@ -614,8 +742,7 @@ void ModbusServerHub::process_modbus_client_frame_(uint8_t address, uint8_t func // from the values it just stored. status = device->on_write_registers(write_start_address, write_registers); } - if (status.has_value()) { - this->send_exception_(address, function_code, status.value()); + if (this->rejected_(address, function_code, status)) { return; } RegisterValues registers; @@ -632,9 +759,7 @@ void ModbusServerHub::process_modbus_client_frame_(uint8_t address, uint8_t func this->send_exception_(address, function_code, ExceptionCode::ILLEGAL_FUNCTION); return; } - if (status.has_value()) { - this->send_exception_(address, function_code, status.value()); - } else { + if (!this->rejected_(address, function_code, status)) { this->send_response_(address, function_code, response_data, response_len); } } @@ -733,6 +858,19 @@ void ModbusServerHub::send_response_(uint8_t address, uint8_t function_code, con this->send_raw_(raw_frame, payload_len + 2); } +bool ModbusServerHub::rejected_(uint8_t address, uint8_t function_code, ResponseStatus status) { + if (!status.has_value()) + return false; + // The one place a rejection becomes an exception reply, so the log carries the transaction context a + // device handler never has: which client-facing address and function code drew which exception. DEBUG + // rather than WARN because an exception reply is a normal protocol outcome and arrives per frame - a + // probing or broken client would otherwise flood the log. The parse helpers still WARN with specifics. + ESP_LOGD(TAG, "Exception %" PRIu8 " replied to function 0x%02X for address %" PRIu8, + static_cast(status.value()), function_code, address); + this->send_exception_(address, function_code, status.value()); + return true; +} + void ModbusServerHub::send_exception_(uint8_t address, uint8_t function_code, ExceptionCode exception_code) { uint8_t raw_frame[3]; raw_frame[0] = address; @@ -918,7 +1056,8 @@ bool ModbusClientHub::queue_pdu(uint8_t address, std::span pdu, M continue; if (device == nullptr) { // A dropped read is routine (DEBUG); a dropped write/custom warns (unobservable without a device). - const bool requeueable = !helpers::is_function_code_exception(pdu[0]) && helpers::is_function_code_read(pdu[0]); + const bool requeueable = + !helpers::is_function_code_exception(pdu[0]) && helpers::is_function_code_read_only(pdu[0]); if (requeueable) { ESP_LOGD(TAG, "Anonymous duplicate of active frame for %" PRIu8 " (function 0x%X), dropped", address, pdu[0]); } else { @@ -1098,7 +1237,14 @@ void ModbusClientDevice::dispatch_response_(std::span request_pdu switch (function_code) { case FunctionCode::READ_HOLDING_REGISTERS: - case FunctionCode::READ_INPUT_REGISTERS: { + case FunctionCode::READ_INPUT_REGISTERS: + // FC 0x17 lands here too: its read start address and read quantity sit at the same request offsets as a + // plain read's (bytes 1..2 and 3..4), so start_address and count_or_value already hold the read block; its + // response carries only that read data, and the write half is confirmed by the response arriving at all. + // An exception routes here as well (the gate only validates the request when status is set), delivering + // empty registers with the error in status - so a 0x17 subclass handles success and failure in the one + // on_read_holding_registers() callback and never needs to also override on_error(). + case FunctionCode::READ_WRITE_MULTIPLE_REGISTERS: { // Decode the big-endian register words into host byte order. The gate guarantees a success response // carries exactly count_or_value registers (and count_or_value <= MAX_NUM_OF_REGISTERS_TO_READ, the // capacity of RegisterValues); a mismatch was diverted to on_custom_response(), never clamped. On @@ -1110,10 +1256,15 @@ void ModbusClientDevice::dispatch_response_(std::span request_pdu } } std::span register_span(registers.data(), registers.size()); - if (function_code == FunctionCode::READ_HOLDING_REGISTERS) { + if (function_code == FunctionCode::READ_INPUT_REGISTERS) { + this->on_read_input_registers(start_address, register_span, status); + } else if (function_code == FunctionCode::READ_HOLDING_REGISTERS || + function_code == FunctionCode::READ_WRITE_MULTIPLE_REGISTERS) { this->on_read_holding_registers(start_address, register_span, status); } else { - this->on_read_input_registers(start_address, register_span, status); + // Unreachable for the current case labels; match explicitly so a function code added to this group + // later is diverted to on_custom_response() rather than silently delivered as a holding read. + this->on_custom_response(request_pdu, response_pdu, status); } break; } diff --git a/esphome/components/modbus/modbus.h b/esphome/components/modbus/modbus.h index 3b6028e90a..6bd407a687 100644 --- a/esphome/components/modbus/modbus.h +++ b/esphome/components/modbus/modbus.h @@ -151,9 +151,7 @@ struct ModbusDeviceCommand { static CommandPriority classify(uint8_t function_code) { if (helpers::is_function_code_exception(function_code)) return CommandPriority::READ; - const auto code = static_cast(function_code); - if (helpers::is_function_code_write(function_code) || code == FunctionCode::MASK_WRITE_REGISTER || - code == FunctionCode::READ_WRITE_MULTIPLE_REGISTERS) { + if (helpers::is_function_code_write(function_code)) { return CommandPriority::WRITE; } return CommandPriority::READ; @@ -162,7 +160,7 @@ struct ModbusDeviceCommand { // Requests this entry can serve: a standard read twice (run plus one re-run), everything else once. uint8_t max_pending() const { const uint8_t fc = this->frame.pdu()[0]; - const bool requeueable = !helpers::is_function_code_exception(fc) && helpers::is_function_code_read(fc); + const bool requeueable = !helpers::is_function_code_exception(fc) && helpers::is_function_code_read_only(fc); return (requeueable && !this->continuous) ? 2 : 1; } // Device-scoped clear: detach with no callback (device-less, pending 0). An entry still waiting for @@ -361,9 +359,27 @@ class ModbusServerHub : public Modbus { // Appends the big-endian register values in values to registers, in host byte order. void assemble_registers_(std::span values, RegisterValues ®isters); ModbusServerDevice *find_device_(uint8_t address); - // Returns std::nullopt if [start_address, start_address + number_of_registers) fits in the 16-bit address space, - // otherwise ILLEGAL_DATA_ADDRESS. The caller sends the exception reply if one is required. - ResponseStatus check_register_range_(uint16_t start_address, uint16_t number_of_registers); + // Returns std::nullopt if [start_address, start_address + count) fits in the 16-bit address space, + // otherwise ILLEGAL_DATA_ADDRESS. The caller sends the exception reply if one is required - a broadcast + // write is never answered, so the check cannot send it itself. Shared by the register and + // coil/discrete-input handlers, which all address the same 16-bit space. + ResponseStatus check_address_range_(uint16_t start_address, uint16_t count); + + // Parses a read request PDU (start address(2) + quantity(2)), shared by the register and + // coil/discrete-input reads so the two cannot drift apart. max_entities is the protocol ceiling for the + // function code; entity_name only labels the rejection log. + ResponseStatus parse_read_request_(std::span data, uint16_t max_entities, const LogString *entity_name, + uint16_t &start_address, uint16_t &count); + + // Parses a single-coil write PDU (FC 0x05), which carries a 2-byte on/off value rather than packed + // bytes. The caller packs value into a byte it owns to build the PackedBits view the handlers take. + ResponseStatus parse_write_single_coil_(std::span data, uint16_t &start_address, bool &value); + + // Parses a multiple-coil write PDU (FC 0x0F) into a packed-bit view pointing straight into the receive + // buffer, so the coil values are never copied. Both coil parsers are shared by the addressed and + // broadcast paths so the two validate identically. + ResponseStatus parse_write_multiple_coils_(std::span data, uint16_t &start_address, uint16_t &count, + std::span &packed_bytes); // Builds the body of a register read response (byte count followed by the big-endian register values) into // response_buffer. Shared by every function code that answers with register values, so the read reply stays @@ -374,6 +390,9 @@ class ModbusServerHub : public Modbus { uint16_t number_of_registers, const RegisterValues ®isters, std::span response_buffer, uint16_t &response_len); void send_raw_(const uint8_t *payload, uint16_t len); + // Sends and logs the exception reply when status holds one; returns true if the request was rejected. + // Every parse and handler rejection funnels through here, so the reply and its log cannot drift apart. + bool rejected_(uint8_t address, uint8_t function_code, ResponseStatus status); void send_exception_(uint8_t address, uint8_t function_code, ExceptionCode exception_code); void send_response_(uint8_t address, uint8_t function_code, const uint8_t *payload, uint16_t payload_len); uint8_t expecting_peer_response_{0}; @@ -573,6 +592,16 @@ class ModbusClientDevice { bool write_multiple_coils(uint16_t start_address, PackedBits bits) { return this->queue_pdu(helpers::create_write_coils_pdu(start_address, bits)); } + /// FC 0x17: the read-back is delivered through on_read_holding_registers() (the response carries only the + /// read registers, the same wire shape as a holding-register read). A device exception - typically a + /// rejected write half - arrives at that same on_read_holding_registers() with the error in its status, + /// exactly as success does, so a subclass overriding that one callback handles both outcomes and never + /// needs to also override on_error(). + bool read_write_multiple_registers(uint16_t read_start_address, uint16_t read_count, uint16_t write_start_address, + std::span write_values) { + return this->queue_pdu(helpers::create_read_write_multiple_registers_pdu(read_start_address, read_count, + write_start_address, write_values)); + } inline void clear_tx_queue_for_address() { this->parent_->clear_tx_queue_for_address(this->address_); } inline void clear_tx_queue_for_device() { this->parent_->clear_tx_queue_for_device(this); } @@ -644,18 +673,26 @@ class ModbusServerDevice { virtual ResponseStatus on_write_registers(uint16_t start_address, const RegisterValues ®isters) { return ExceptionCode::ILLEGAL_FUNCTION; }; - // Hub entry point for broadcast (address 0) writes, which are never answered. - ResponseStatus on_broadcast_write_registers(uint16_t start_address, const RegisterValues ®isters) { - this->broadcast_write_ = true; - ResponseStatus status = this->on_write_registers(start_address, registers); - this->broadcast_write_ = false; - return status; - } + /// Coil/discrete-input reads: set the requested bits (bit 0 = the coil at start_address) with + /// bits.set(). The view covers bits.size() pre-zeroed bits and writes land directly in the hub's + /// response buffer (no copy); it is only valid during the call. + virtual ResponseStatus on_read_bits(uint16_t start_address, MutablePackedBits bits) { + return ExceptionCode::ILLEGAL_FUNCTION; + }; + virtual ResponseStatus on_read_coils(uint16_t start_address, MutablePackedBits bits) { + return this->on_read_bits(start_address, bits); + }; + virtual ResponseStatus on_read_discrete_inputs(uint16_t start_address, MutablePackedBits bits) { + return this->on_read_bits(start_address, bits); + }; + /// Coil writes deliver the values as a PackedBits view over the hub's receive buffer (only valid + /// during the call). A single-coil write (FC 0x05) arrives as bits.size() == 1. + virtual ResponseStatus on_write_coils(uint16_t start_address, PackedBits bits) { + return ExceptionCode::ILLEGAL_FUNCTION; + }; protected: uint8_t address_{0}; - // Set while handling a broadcast write: the caller sends no reply, so a rejection has no wire consequence. - bool broadcast_write_{false}; }; } // namespace esphome::modbus diff --git a/esphome/components/modbus/modbus_definitions.h b/esphome/components/modbus/modbus_definitions.h index 9ec776b67a..64f7210585 100644 --- a/esphome/components/modbus/modbus_definitions.h +++ b/esphome/components/modbus/modbus_definitions.h @@ -128,6 +128,19 @@ static_assert(MAX_RAW_SIZE + 2 == MAX_FRAME_SIZE, "a framed raw server payload m /// Bits pack 8 per data byte, rounded up to whole bytes. constexpr size_t packed_bit_bytes(size_t bits) { return (bits + 7) / 8; } +// A coil/discrete-input read answers with byte count(1) + packed_bit_bytes(count) bytes, which has to fit +// the raw frame body. The runtime check on that path catches a caller entering with bytes already written; +// this catches the other way in, raising the ceiling past what a frame can carry. +static_assert(1 + packed_bit_bytes(MAX_NUM_OF_COILS_TO_READ) <= MAX_RAW_SIZE, + "MAX_NUM_OF_COILS_TO_READ yields a read response larger than MAX_RAW_SIZE"); +static_assert(1 + packed_bit_bytes(MAX_NUM_OF_DISCRETE_INPUTS_TO_READ) <= MAX_RAW_SIZE, + "MAX_NUM_OF_DISCRETE_INPUTS_TO_READ yields a read response larger than MAX_RAW_SIZE"); + +// The coil and discrete-input ceilings are separate limits in the spec but hold the same value, so the +// read paths validate both against MAX_NUM_OF_COILS_TO_READ. Should the spec ever split them, this fires. +static_assert(MAX_NUM_OF_COILS_TO_READ == MAX_NUM_OF_DISCRETE_INPUTS_TO_READ, + "the coil and discrete-input read ceilings must match"); + /** Read-only view of Modbus-packed bits: bit 0 of byte 0 is the first bit (LSB first), the layout * coil/discrete-input values use on the wire. Bundles the bit count with the packed bytes so the * two cannot desynchronize. The view does not own the bytes - it is only valid while they are. diff --git a/esphome/components/modbus/modbus_helpers.cpp b/esphome/components/modbus/modbus_helpers.cpp index a0c8440c79..db21b6e6fd 100644 --- a/esphome/components/modbus/modbus_helpers.cpp +++ b/esphome/components/modbus/modbus_helpers.cpp @@ -8,10 +8,11 @@ namespace esphome::modbus::helpers { static const char *const TAG = "modbus_helpers"; // A quantity/address pair is standard when the quantity is non-zero, within the per-table maximum, -// and the range [start_address, start_address + quantity) stays inside the 16-bit address space -// (the 32-bit promotion is the overflow guard - a 16-bit sum could wrap and pass). +// and the range [start_address, start_address + quantity) stays inside the 16-bit address space. +// Non-logging twin of register_block_in_range(): the same three predicates for the parser side, taking a +// uint16_t quantity. register_block_in_range() is the builder-side variant that also logs which half failed. static bool quantity_in_range(uint16_t start_address, uint16_t quantity, uint16_t max_quantity) { - return quantity != 0 && quantity <= max_quantity && uint32_t(start_address) + quantity <= 0x10000u; + return quantity != 0 && quantity <= max_quantity && address_range_fits(start_address, quantity); } // The spec allows exactly ON (0xFF00) and OFF (0x0000) for a single-coil value, on the request and @@ -69,8 +70,10 @@ uint16_t client_pdu_length(const uint8_t *frame, size_t size) { case FunctionCode::WRITE_SINGLE_REGISTER: return 5; // function(1) + output/register address(2) + value(2) case FunctionCode::WRITE_MULTIPLE_COILS: + // function(1) + start address(2) + quantity(2) + byte count(1) + packed coil data (8 coils per byte). + return 6 + (size > 5 ? std::min(frame[5], uint8_t(packed_bit_bytes(MAX_NUM_OF_COILS_TO_WRITE))) : 0); case FunctionCode::WRITE_MULTIPLE_REGISTERS: - // function(1) + start address(2) + quantity(2) + byte count(1) + data + // function(1) + start address(2) + quantity(2) + byte count(1) + register data (2 bytes per register). return 6 + (size > 5 ? std::min(frame[5], uint8_t(MAX_NUM_OF_REGISTERS_TO_WRITE * 2)) : 0); // Unsupported function codes. Included here to prevent parser failures. Excluding Serial Line specific functions. case FunctionCode::READ_FILE_RECORD: @@ -305,16 +308,20 @@ std::optional registers_to_number(const uint16_t *registers, size_t cou return payload_to_number(bytes, required_size, sensor_value_type, 0, 0xFFFFFFFF); } +// Append a 16-bit value to a PDU in big-endian (wire) byte order. +template static void append_pdu_word(StaticVector &pdu, uint16_t value) { + pdu.push_back(value >> 8); + pdu.push_back(value >> 0); +} + // Every request PDU opens with the same 5-byte layout: function code, then two big-endian 16-bit // fields (start address + quantity for reads and multi-writes, address + value for single writes). template static void append_pdu_header(StaticVector &pdu, FunctionCode function_code, uint16_t first, uint16_t second) { pdu.push_back(static_cast(function_code)); - pdu.push_back(first >> 8); - pdu.push_back(first >> 0); - pdu.push_back(second >> 8); - pdu.push_back(second >> 0); + append_pdu_word(pdu, first); + append_pdu_word(pdu, second); } // Zero the unused bits of a multi-coil write's final data byte, as the spec requires. Kept in one @@ -333,7 +340,7 @@ ReadPdu create_read_pdu(FunctionCode function_code, uint16_t start_address, uint ESP_LOGE(TAG, "Number of entities is zero for function code %02X", static_cast(function_code)); return pdu; } - if (uint32_t(start_address) + number_of_entities > 0x10000u) { + if (!address_range_fits(start_address, number_of_entities)) { ESP_LOGE(TAG, "Read of %u entities at %u runs past the 16-bit address space, dropping request", number_of_entities, start_address); return pdu; @@ -376,7 +383,7 @@ PduBuffer create_client_pdu(FunctionCode function_code, uint16_t start_address, PduBuffer pdu; // declared before every return so NRVO fires (all paths return the same object) // Generic entry point; prefer the direction- and type-specific builders (create_read_pdu(), // create_write_registers_pdu(), etc.) which bound their inputs per spec. - if (is_function_code_read(static_cast(function_code))) { + if (is_function_code_read_only(static_cast(function_code))) { if (values != nullptr || values_len > 0) { ESP_LOGW(TAG, "Values provided for read function code %02X, but will be ignored", static_cast(function_code)); @@ -415,7 +422,7 @@ PduBuffer create_client_pdu(FunctionCode function_code, uint16_t start_address, static_cast(function_code)); return pdu; } - if (!is_single && uint32_t(start_address) + number_of_entities > 0x10000u) { + if (!is_single && !address_range_fits(start_address, number_of_entities)) { ESP_LOGE(TAG, "Write of %u entities at %u runs past the 16-bit address space, dropping request", number_of_entities, start_address); return pdu; @@ -458,29 +465,59 @@ PduBuffer create_client_pdu(FunctionCode function_code, uint16_t start_address, return pdu; } +// Validate one register block for a client builder: a non-zero quantity within max_quantity that does not +// run past the 16-bit address space (register count × 2 stays within MAX_PDU_SIZE as a result). On failure +// it logs the reason and returns false, on which the caller returns an empty PDU. `role` names the block in +// the log ("Read"/"Write"). Logging twin of quantity_in_range(): the same three predicates, split so each +// failure names its reason, and taking size_t so an oversize span is caught before any narrowing. +static bool register_block_in_range(const LogString *role, uint16_t start_address, size_t quantity, + uint16_t max_quantity) { + if (quantity == 0 || quantity > max_quantity) { + ESP_LOGE(TAG, "%s count %zu out of range [1, %u], dropping request", LOG_STR_ARG(role), quantity, max_quantity); + return false; + } + if (!address_range_fits(start_address, quantity)) { + ESP_LOGE(TAG, "%s of %zu registers at %u runs past the 16-bit address space, dropping request", LOG_STR_ARG(role), + quantity, start_address); + return false; + } + return true; +} + PduBuffer create_write_registers_pdu(uint16_t start_address, std::span values) { PduBuffer pdu; // declared before every return so NRVO fires (all paths return the same object) - if (values.empty()) { - ESP_LOGE(TAG, "No values provided for write multiple registers, dropping request"); - return pdu; - } - // Byte count is registers × 2 (per spec); bounding the register count keeps the PDU within MAX_PDU_SIZE. - if (values.size() > MAX_NUM_OF_REGISTERS_TO_WRITE) { - ESP_LOGE(TAG, "values.size() %zu exceeds maximum registers to write %u, dropping request", values.size(), - MAX_NUM_OF_REGISTERS_TO_WRITE); - return pdu; - } - if (uint32_t(start_address) + values.size() > 0x10000u) { - ESP_LOGE(TAG, "Write of %zu registers at %u runs past the 16-bit address space, dropping request", values.size(), - start_address); + if (!register_block_in_range(LOG_STR("Write"), start_address, values.size(), MAX_NUM_OF_REGISTERS_TO_WRITE)) { return pdu; } append_pdu_header(pdu, FunctionCode::WRITE_MULTIPLE_REGISTERS, start_address, values.size()); pdu.push_back(static_cast(values.size() * 2)); // byte count for (auto v : values) { - auto decoded_value = decode_value(v); - pdu.push_back(decoded_value[0]); - pdu.push_back(decoded_value[1]); + append_pdu_word(pdu, v); + } + return pdu; +} + +PduBuffer create_read_write_multiple_registers_pdu(uint16_t read_start_address, uint16_t read_count, + uint16_t write_start_address, + std::span write_values) { + PduBuffer pdu; + if (!register_block_in_range(LOG_STR("Read"), read_start_address, read_count, MAX_NUM_OF_REGISTERS_TO_READ)) { + return pdu; + } + if (!register_block_in_range(LOG_STR("Write"), write_start_address, write_values.size(), + MAX_NUM_OF_REGISTERS_TO_WRITE_RW)) { + return pdu; + } + // fc + read start(2) + read qty(2) + write start(2) + write qty(2) + write byte count(1) + write values. + const auto write_count = static_cast(write_values.size()); + pdu.push_back(static_cast(FunctionCode::READ_WRITE_MULTIPLE_REGISTERS)); + append_pdu_word(pdu, read_start_address); + append_pdu_word(pdu, read_count); + append_pdu_word(pdu, write_start_address); + append_pdu_word(pdu, write_count); + pdu.push_back(static_cast(write_count * 2)); // byte count + for (auto v : write_values) { + append_pdu_word(pdu, v); } return pdu; } @@ -510,7 +547,7 @@ static void build_write_coils_pdu(PduBuffer &pdu, uint16_t start_address, Packed ESP_LOGE(TAG, "count %u exceeds maximum coils to write %u, dropping request", count, MAX_NUM_OF_COILS_TO_WRITE); return; } - if (uint32_t(start_address) + count > 0x10000u) { + if (!address_range_fits(start_address, count)) { ESP_LOGE(TAG, "Write of %u coils at %u runs past the 16-bit address space, dropping request", count, start_address); return; } @@ -546,12 +583,7 @@ static PduBuffer create_write_coils_pdu_from_bools(uint16_t start_address, const return pdu; } CoilPackBuffer packed; - for (size_t i = 0; i != count; i++) { - if (i % 8 == 0) - packed.push_back(0); - if (values[i]) - packed[i / 8] |= (1 << (i % 8)); - } + pack_bits(packed, values); build_write_coils_pdu(pdu, start_address, PackedBits(std::span(packed.data(), packed.size()), count)); return pdu; } diff --git a/esphome/components/modbus/modbus_helpers.h b/esphome/components/modbus/modbus_helpers.h index 2c312b8a61..c737e206c0 100644 --- a/esphome/components/modbus/modbus_helpers.h +++ b/esphome/components/modbus/modbus_helpers.h @@ -11,7 +11,8 @@ namespace esphome::modbus::helpers { -inline bool is_function_code_read(uint8_t function_code) { +// Pure read codes (0x01-0x04): they only read, so they are idempotent and safe to retry. +inline bool is_function_code_read_only(uint8_t function_code) { FunctionCode masked_function_code = static_cast(function_code & FUNCTION_CODE_MASK); return masked_function_code == FunctionCode::READ_COILS || masked_function_code == FunctionCode::READ_DISCRETE_INPUTS || @@ -19,12 +20,27 @@ inline bool is_function_code_read(uint8_t function_code) { masked_function_code == FunctionCode::READ_INPUT_REGISTERS; } +// Codes whose response carries read-back data: the pure reads plus 0x17, which reads and writes at once. +inline bool is_function_code_read(uint8_t function_code) { + return is_function_code_read_only(function_code) || + static_cast(function_code & FUNCTION_CODE_MASK) == FunctionCode::READ_WRITE_MULTIPLE_REGISTERS; +} + +// Codes that mutate registers or coils: the pure writes, 0x16 mask-write, and 0x17 read/write multiple. inline bool is_function_code_write(uint8_t function_code) { FunctionCode masked_function_code = static_cast(function_code & FUNCTION_CODE_MASK); return masked_function_code == FunctionCode::WRITE_SINGLE_COIL || masked_function_code == FunctionCode::WRITE_SINGLE_REGISTER || masked_function_code == FunctionCode::WRITE_MULTIPLE_COILS || - masked_function_code == FunctionCode::WRITE_MULTIPLE_REGISTERS; + masked_function_code == FunctionCode::WRITE_MULTIPLE_REGISTERS || + masked_function_code == FunctionCode::MASK_WRITE_REGISTER || + masked_function_code == FunctionCode::READ_WRITE_MULTIPLE_REGISTERS; +} + +// True if [start_address, start_address + count) fits within the 16-bit Modbus address space. The 32-bit +// promotion is the overflow guard - a 16-bit sum could wrap and pass. +inline bool address_range_fits(uint16_t start_address, size_t count) { + return uint32_t(start_address) + count <= 0x10000u; } inline bool is_function_code_exception(uint8_t function_code) { @@ -90,8 +106,8 @@ inline uint8_t server_frame_data_offset(const uint8_t *frame, size_t size) { } /** Returns the payload portion of a server response PDU: the bytes after the function code, and for the - * standard read responses (0x01-0x04) also after the byte-count byte. Responses to 0x14/0x17 also carry a - * byte-count byte, but those codes are not implemented and their count byte is left in the payload. For + * read responses (0x01-0x04 and 0x17) also after the byte-count byte. Response 0x14 also carries a + * byte-count byte, but that code is not implemented and its count byte is left in the payload. For * an exception PDU the payload is the exception code byte (the read check must not see the masked * function code, or an exception-of-read would classify as a read and return an empty span). Returns an * empty span if the PDU is too short. @@ -257,6 +273,29 @@ inline bool bit_from_packed(int bit, std::span data) { ESPDEPRECATED("Use bit_from_packed() instead. Removed in 2027.2.0", "2026.8.0") inline bool coil_from_vector(int coil, std::span data) { return bit_from_packed(coil, data); } +/** Append packed bytes (LSB first) for the given bits onto a growable byte container. + * push_back-based so callers can build a payload incrementally (e.g. a std::vector + * with no fixed upper bound). A non-byte-aligned count appends n+1 bytes, the last holding + * the remaining bits in its low positions. + * @param out destination byte container exposing push_back(uint8_t) + * @param bits container of bool exposing range-based iteration + */ +template void pack_bits(Out &out, const Bits &bits) { + uint8_t byte = 0; + uint8_t bit = 0; + for (bool b : bits) { + if (b) + byte |= (1 << bit); + if (++bit == 8) { + out.push_back(byte); + byte = 0; + bit = 0; + } + } + if (bit != 0) // flush the final partial byte + out.push_back(byte); +} + /** Extract bits from value and shift right according to the bitmask * if the bitmask is 0x00F0 we want the values frrom bit 5 - 8. * the result is then shifted right by the position if the first right set bit in the mask @@ -409,6 +448,21 @@ PduBuffer create_client_pdu(FunctionCode function_code, uint16_t start_address, */ PduBuffer create_write_registers_pdu(uint16_t start_address, std::span values); +/** Create modbus read/write multiple registers command + * Function 0x17 Read/Write Multiple Registers + * Writes write_values then reads read_count registers in one transaction (write first, per Modbus 6.17); + * the response carries only the read registers. + * @param read_start_address modbus address of the first register to read back + * @param read_count number of registers to read (at most MAX_NUM_OF_REGISTERS_TO_READ) + * @param write_start_address modbus address of the first register to write + * @param write_values register values to write; the register count is write_values.size() (at most + * MAX_NUM_OF_REGISTERS_TO_WRITE_RW). Any contiguous uint16_t container converts. + * @return PDU (function code + data, no address, no CRC); an empty PDU on any out-of-range input + */ +PduBuffer create_read_write_multiple_registers_pdu(uint16_t read_start_address, uint16_t read_count, + uint16_t write_start_address, + std::span write_values); + /** Create modbus write single register command * Function 0x06 Write Single Register * @param start_address modbus address of the register to write diff --git a/esphome/components/modbus_client/__init__.py b/esphome/components/modbus_client/__init__.py index 52a61cacad..bb113d649c 100644 --- a/esphome/components/modbus_client/__init__.py +++ b/esphome/components/modbus_client/__init__.py @@ -28,9 +28,12 @@ CONF_ON_NO_RESPONSE = "on_no_response" CONF_ON_NOT_SENT = "on_not_sent" CONF_ON_SENT = "on_sent" CONF_PDU = "pdu" +CONF_READ_ADDRESS = "read_address" +CONF_READ_COUNT = "read_count" CONF_RETRY = "retry" CONF_START_ADDRESS = "start_address" CONF_VALUES = "values" +CONF_WRITE_ADDRESS = "write_address" modbus_client_ns = cg.esphome_ns.namespace("modbus_client") ModbusClientSendAction = modbus_client_ns.class_( @@ -55,6 +58,9 @@ WriteMultipleRegistersAction = modbus_client_ns.class_( WriteMultipleCoilsAction = modbus_client_ns.class_( "WriteMultipleCoilsAction", automation.Action, modbus.ModbusClientDevice ) +ReadWriteMultipleRegistersAction = modbus_client_ns.class_( + "ReadWriteMultipleRegistersAction", automation.Action, modbus.ModbusClientDevice +) # Packed bit view delivered to read_coils / read_discrete_inputs on_response handlers. PackedBits = modbus.modbus_ns.class_("PackedBits") @@ -255,21 +261,30 @@ async def modbus_client_send_to_code(config, action_id, template_arg, args): _REGISTER_SPAN = cg.std_span.template(cg.uint16.operator("const")) -# Every typed action addresses a register or coil range and reports through the same two reply handlers. -_TYPED_ACTION_SCHEMA = _ACTION_BASE_SCHEMA.extend( +# The reply-handler pair every typed-dispatch action reports through. Kept in one place so the +# read/write-multiple schema (which cannot require start_address) shares it instead of drifting. +# Both use _handler_schema(): the decoded arguments (values span, bits view) point at buffers the hub +# reuses once the handler returns, so a deferring action would resume on freed memory. A reply the +# dispatch gate diverts (not a standard-conformant transaction) arrives at on_custom_response with the +# raw request/response PDUs; real device exceptions still arrive via on_error. +_REPLY_HANDLERS_SCHEMA = cv.Schema( { - cv.Required(CONF_START_ADDRESS): cv.templatable(cv.hex_uint16_t), - # Both use _handler_schema(): the decoded arguments (values span, bits view) point at buffers the - # hub reuses once the handler returns, so a deferring action would resume on freed memory. cv.Optional(CONF_ON_RESPONSE): _handler_schema(), - # A reply the dispatch gate diverts (not a standard-conformant transaction) arrives here with the - # raw request/response PDUs; real device exceptions still arrive via on_error. cv.Optional(CONF_ON_CUSTOM_RESPONSE): _handler_schema(), } ) +# Every typed action addresses a register or coil range and reports through the shared reply handlers. +_TYPED_ACTION_SCHEMA = _ACTION_BASE_SCHEMA.extend(_REPLY_HANDLERS_SCHEMA).extend( + { + cv.Required(CONF_START_ADDRESS): cv.templatable(cv.hex_uint16_t), + } +) -def _no_address_overflow(count_key: str) -> Callable[[ConfigType], ConfigType]: + +def _no_address_overflow( + count_key: str, address_key: str = CONF_START_ADDRESS +) -> Callable[[ConfigType], ConfigType]: """Reject a range that runs past the 16-bit address space, which the device could never answer. Only literal configurations can be checked: either operand may be a lambda, and its value is not known @@ -278,17 +293,17 @@ def _no_address_overflow(count_key: str) -> Callable[[ConfigType], ConfigType]: """ def validate(config: ConfigType) -> ConfigType: - start = config[CONF_START_ADDRESS] + start = config[address_key] count = config[count_key] if isinstance(start, Lambda) or isinstance(count, Lambda): return config - # CONF_COUNT is a number; CONF_VALUES is the list whose length is the count. + # A count key holds a number; a values key holds the list whose length is the count. length = count if isinstance(count, int) else len(count) if start + length > 0x10000: raise cv.Invalid( - f"{CONF_START_ADDRESS} 0x{start:04X} plus {length} entities runs past the end of the " + f"{address_key} 0x{start:04X} plus {length} entities runs past the end of the " f"16-bit address space (last addressable entity is 0xFFFF)", - path=[CONF_START_ADDRESS], + path=[address_key], ) return config @@ -468,3 +483,64 @@ async def write_multiple_coils_to_code(config, action_id, template_arg, args): arr = cg.static_const_array(arr_id, cg.ArrayInitializer(*packed)) cg.add(var.set_values_static(arr, len(values))) return await register_client_action(var, config, args, []) + + +# Read/write multiple registers (FC 0x17) writes one register block and reads another in a single +# transaction, so it has two address ranges and uses read_address/write_address instead of start_address. +# Note the two meanings of `values`: here it is the block being WRITTEN, while in on_response the lambda +# argument `values` is the block that was READ BACK (host-order words, the same shape as +# read_holding_registers, so a caller can feed it through the same handler). +_READ_WRITE_MULTIPLE_REGISTERS_SCHEMA = cv.All( + _ACTION_BASE_SCHEMA.extend(_REPLY_HANDLERS_SCHEMA).extend( + { + cv.Required(CONF_READ_ADDRESS): cv.templatable(cv.hex_uint16_t), + cv.Optional(CONF_READ_COUNT, default=1): cv.templatable( + cv.int_range(min=1, max=modbus.MAX_NUM_OF_REGISTERS_TO_READ) + ), + cv.Required(CONF_WRITE_ADDRESS): cv.templatable(cv.hex_uint16_t), + cv.Required(CONF_VALUES): cv.templatable( + cv.All( + cv.ensure_list(cv.hex_uint16_t), + cv.Length(min=1, max=modbus.MAX_NUM_OF_REGISTERS_TO_WRITE_RW), + ) + ), + } + ), + _no_address_overflow(CONF_READ_COUNT, CONF_READ_ADDRESS), + _no_address_overflow(CONF_VALUES, CONF_WRITE_ADDRESS), +) + + +@automation.register_action( + "modbus_client.read_write_multiple_registers", + ReadWriteMultipleRegistersAction, + _READ_WRITE_MULTIPLE_REGISTERS_SCHEMA, + synchronous=True, +) +async def read_write_multiple_registers_to_code(config, action_id, template_arg, args): + var = cg.new_Pvariable(action_id, template_arg) + cg.add( + var.set_read_address( + await cg.templatable(config[CONF_READ_ADDRESS], args, cg.uint16) + ) + ) + cg.add( + var.set_read_count( + await cg.templatable(config[CONF_READ_COUNT], args, cg.uint16) + ) + ) + cg.add( + var.set_write_address( + await cg.templatable(config[CONF_WRITE_ADDRESS], args, cg.uint16) + ) + ) + values = config[CONF_VALUES] + if cg.is_template(values): + templ = await cg.templatable(values, args, cg.std_vector.template(cg.uint16)) + cg.add(var.set_values_template(templ)) + else: + # A static list goes to flash, so play() sends straight from there without allocating. + arr_id = ID(f"{action_id}_values", is_declaration=True, type=cg.uint16) + arr = cg.static_const_array(arr_id, cg.ArrayInitializer(*values)) + cg.add(var.set_values_static(arr, len(values))) + return await register_client_action(var, config, args, [(_REGISTER_SPAN, "values")]) diff --git a/esphome/components/modbus_client/modbus_client.h b/esphome/components/modbus_client/modbus_client.h index f9a00d65f6..7e6d9d069f 100644 --- a/esphome/components/modbus_client/modbus_client.h +++ b/esphome/components/modbus_client/modbus_client.h @@ -332,4 +332,56 @@ template class WriteMultipleCoilsAction : public TypedClientActi } values_; }; +/// modbus_client.read_write_multiple_registers (FC 0x17): writes one register block and reads another back in +/// one transaction (write first, per Modbus 6.17). on_response delivers the read-back words as `values`. +template class ReadWriteMultipleRegistersAction : public TypedClientActionBase { + public: + TEMPLATABLE_VALUE(uint16_t, read_address) + TEMPLATABLE_VALUE(uint16_t, read_count) + TEMPLATABLE_VALUE(uint16_t, write_address) + + /// Static config: the write registers live in flash, so play() neither allocates nor copies. + void set_values_static(const uint16_t *values, size_t len) { + this->values_.data = values; + this->len_ = static_cast(len); + } + /// Lambda config: the write registers are only known at play() time. + void set_values_template(std::vector (*func)(Ts...)) { + this->values_.func = func; + this->len_ = -1; // sentinel: template mode + } + + Trigger> *get_response_trigger() { return &this->response_trigger_; } + + void play(const Ts &...x) override { + const uint16_t read_start = this->read_address_.value(x...); + const uint16_t read_count = this->read_count_.value(x...); + const uint16_t write_start = this->write_address_.value(x...); + // An out-of-range read/write count builds an empty PDU (the builder logs why), resolving via on_not_sent. + if (this->len_ >= 0) { + this->send_or_resolve_(modbus::helpers::create_read_write_multiple_registers_pdu( + read_start, read_count, write_start, + std::span(this->values_.data, static_cast(this->len_)))); + return; + } + const std::vector values = this->values_.func(x...); + this->send_or_resolve_(modbus::helpers::create_read_write_multiple_registers_pdu( + read_start, read_count, write_start, std::span(values))); + } + // The 0x17 response carries only the read block, so the hub dispatch delivers it as a holding-register read. + void on_read_registers(modbus::EntityType entity_type, uint16_t start_address, std::span registers, + modbus::ResponseStatus status) override { + if (modbus::succeeded(status)) + this->response_trigger_.trigger(registers); + } + + protected: + Trigger> response_trigger_; + ssize_t len_{-1}; // -1 = template mode, >= 0 = static mode with this many write registers + union Values { + std::vector (*func)(Ts...); + const uint16_t *data; + } values_; +}; + } // namespace esphome::modbus_client diff --git a/esphome/components/modbus_controller/modbus_controller.cpp b/esphome/components/modbus_controller/modbus_controller.cpp index da9d29887e..35f21fd0af 100644 --- a/esphome/components/modbus_controller/modbus_controller.cpp +++ b/esphome/components/modbus_controller/modbus_controller.cpp @@ -2,6 +2,8 @@ #include "esphome/core/application.h" #include "esphome/core/log.h" +#include + namespace esphome::modbus_controller { static const char *const TAG = "modbus_controller"; @@ -427,14 +429,15 @@ ModbusCommandItem ModbusCommandItem::create_write_multiple_coils(ModbusControlle modbusdevice->on_write_register_response(register_type, start_address, data); }; - uint8_t *p = cmd.payload.init((values.size() + 7) / 8); - memset(p, 0, (values.size() + 7) / 8); - size_t bit = 0; - for (auto coil : values) { - if (coil) { - p[bit / 8] |= (1 << (bit % 8)); - } - bit++; + // Pack through the shared bit view (MutablePackedBits) so the coil wire layout lives in one place + // instead of an open-coded loop. + const size_t byte_count = modbus::packed_bit_bytes(values.size()); + uint8_t *p = cmd.payload.init(byte_count); + memset(p, 0, byte_count); + modbus::MutablePackedBits bits(std::span(p, byte_count), static_cast(values.size())); + for (size_t i = 0; i != values.size(); i++) { + if (values[i]) + bits.set(i, true); } return cmd; } diff --git a/esphome/components/modbus_server/modbus_server.cpp b/esphome/components/modbus_server/modbus_server.cpp index bf39efbd54..e63495cb25 100644 --- a/esphome/components/modbus_server/modbus_server.cpp +++ b/esphome/components/modbus_server/modbus_server.cpp @@ -145,12 +145,9 @@ modbus::ResponseStatus ModbusServer::on_write_registers(uint16_t start_address, } return true; })) { - // On a broadcast every device that does not map these registers rejects them, which is the normal case. - if (this->broadcast_write_) { - ESP_LOGV(TAG, "Write request rejected before applying any register."); - } else { - ESP_LOGW(TAG, "Write request rejected before applying any register."); - } + // Only VERBOSE: one handler serves both addressed and broadcast writes, and rejecting a broadcast for + // registers this device does not map is routine. The hub logs the outcome with the context it has. + ESP_LOGV(TAG, "Write request rejected before applying any register."); return precheck; } diff --git a/esphome/components/ota/ota_backend.h b/esphome/components/ota/ota_backend.h index 01be46a518..aa93df60a5 100644 --- a/esphome/components/ota/ota_backend.h +++ b/esphome/components/ota/ota_backend.h @@ -4,6 +4,8 @@ #include "esphome/core/defines.h" #include "esphome/core/helpers.h" +#include +#include #include #ifdef USE_OTA_STATE_LISTENER @@ -78,6 +80,25 @@ enum OTAType : uint8_t { OTA_TYPE_UPDATE_BOOTLOADER = 0x02, }; +// The OTA backend method surface. Exactly one backend exists per build, +// selected in ota_backend_factory.h where this concept is asserted on +// make_ota_backend()'s return type. Semantics beyond the signatures: +// - begin: prepare for an image of the given size; ota_type defaults to an +// app update, so both call forms must be accepted. +// - set_update_md5: expected digest of the incoming image, hex string. +// - write: consume the next chunk; end: finalize and mark bootable. +// - abort: safe to call in any state, including after end(). +template +concept OTABackendContract = requires(T backend, size_t image_size, uint8_t *data, size_t len, const char *md5) { + { backend.begin(image_size, OTA_TYPE_UPDATE_APP) } -> std::same_as; + { backend.begin(image_size) } -> std::same_as; + backend.set_update_md5(md5); + { backend.write(data, len) } -> std::same_as; + { backend.end() } -> std::same_as; + backend.abort(); + { backend.supports_compression() } -> std::same_as; +}; + /** Listener interface for OTA state changes. * * Components can implement this interface to receive OTA state updates diff --git a/esphome/components/ota/ota_backend_factory.h b/esphome/components/ota/ota_backend_factory.h index c543983d8d..82d001ed9e 100644 --- a/esphome/components/ota/ota_backend_factory.h +++ b/esphome/components/ota/ota_backend_factory.h @@ -17,11 +17,22 @@ #else // Stub for static analysis when no platform is defined namespace esphome::ota { -struct StubOTABackend {}; +struct StubOTABackend { + OTAResponseTypes begin(size_t image_size, OTAType ota_type = OTA_TYPE_UPDATE_APP) { + return OTA_RESPONSE_ERROR_UNKNOWN; + } + void set_update_md5(const char *md5) {} + OTAResponseTypes write(uint8_t *data, size_t len) { return OTA_RESPONSE_ERROR_UNKNOWN; } + OTAResponseTypes end() { return OTA_RESPONSE_ERROR_UNKNOWN; } + void abort() {} + bool supports_compression() { return false; } +}; std::unique_ptr make_ota_backend(); } // namespace esphome::ota #endif namespace esphome::ota { using OTABackendPtr = decltype(make_ota_backend()); +static_assert(OTABackendContract, + "The platform's OTA backend is missing part of the backend surface (ota_backend.h)"); } // namespace esphome::ota diff --git a/esphome/components/rp2_ble_tracker/__init__.py b/esphome/components/rp2_ble_tracker/__init__.py index 99262babce..7709df9899 100644 --- a/esphome/components/rp2_ble_tracker/__init__.py +++ b/esphome/components/rp2_ble_tracker/__init__.py @@ -41,9 +41,7 @@ RP2BLETracker = rp2_ble_tracker_ns.class_( # to_code(). `active` defaults on for esp32_ble_tracker parity; it adds scan # request TX and roughly doubles the reports through the queue, so # `active: false` is the lighter choice when scan response data is not needed. -SCAN_PARAMETERS_SCHEMA = ble_device_base.scan_parameters_schema( - "100ms", supports_active=True -) +SCAN_PARAMETERS_SCHEMA = ble_device_base.scan_parameters_schema("100ms") CONFIG_SCHEMA = cv.Schema( { diff --git a/esphome/components/ufm01/ufm01.cpp b/esphome/components/ufm01/ufm01.cpp index 1380c34284..bafdb5d853 100644 --- a/esphome/components/ufm01/ufm01.cpp +++ b/esphome/components/ufm01/ufm01.cpp @@ -4,13 +4,23 @@ #include "esphome/core/log.h" #include +#include +#include namespace esphome::ufm01 { static const char *const TAG = "ufm01"; static constexpr uint8_t COMMAND_ACK = 0xE5; -static constexpr uint32_t COMMAND_ACK_TIMEOUT_MS = 200; +static constexpr uint32_t COMMAND_ACK_TIMEOUT_MS = 500; +static constexpr uint32_t STARTUP_DELAY_MS = 2000; +static constexpr uint32_t POST_RESET_DELAY_MS = 2000; +static constexpr uint32_t RESET_RETRY_DELAY_MS = 800; +static constexpr uint32_t STARTUP_RETRY_MS = 3000; +static constexpr uint32_t PASSIVE_POLL_INTERVAL_MS = 1000; +static constexpr uint32_t ACTIVE_STALE_MS = 5000; +static constexpr uint32_t PASSIVE_READ_TIMEOUT_MS = 1000; +static constexpr uint32_t ACTIVE_FRAME_TIMEOUT_MS = 3000; static constexpr float L_PER_M3 = 1000.0f; static constexpr float M3_PER_L = 1.0f / L_PER_M3; @@ -18,12 +28,14 @@ static constexpr float M3_PER_L = 1.0f / L_PER_M3; static constexpr std::array ACTIVE_MODE = {0xFE, 0xFE, 0x11, 0x5C, 0x00, 0x5C, 0x16}; static constexpr std::array CLEAR_ACCUMULATED_FLOW = {0xFE, 0xFE, 0x11, 0x5A, 0xFD, 0x57, 0x16}; static constexpr std::array RESET_DEVICE = {0xFE, 0xFE, 0x11, 0x5D, 0xCB, 0x28, 0x16}; +static constexpr std::array READ_SENSOR_DATA_NO_ID = {0xFE, 0xFE, 0x11, 0x5B, 0x0F, 0x6A, 0x16}; // Active-mode frame layout (datasheet Table 7) static constexpr size_t FRAME_CHECKSUM_INDEX = 30; static constexpr size_t FRAME_STOP_INDEX = 31; static constexpr uint8_t FRAME_START_BYTE_1 = 0x3C; static constexpr uint8_t FRAME_START_BYTE_2 = 0x32; +static constexpr uint8_t PASSIVE_START_BYTE_2 = 0x64; static constexpr uint8_t FRAME_STOP_BYTE = 0x16; static constexpr uint8_t FRAME_INDEX_INSTANT_FLOW_FLAG = 15; static constexpr uint8_t FRAME_INDEX_RESERVED_SECTION = 21; @@ -55,7 +67,7 @@ static bool check_byte(const uint8_t data[FRAME_SIZE], size_t index, uint8_t exp return false; } -static bool validate_data(uint8_t data[FRAME_SIZE]) { +static bool validate_active_frame(const uint8_t data[FRAME_SIZE]) { uint8_t sum = 0; for (size_t i = 0; i < FRAME_CHECKSUM_INDEX; ++i) sum += data[i]; @@ -68,13 +80,43 @@ static bool validate_data(uint8_t data[FRAME_SIZE]) { check_byte(data, FRAME_STOP_INDEX, FRAME_STOP_BYTE, "stop byte"); } -static float read_accumulated_flow(uint8_t data[FRAME_SIZE]) { +static bool validate_passive_frame(const uint8_t data[PASSIVE_FRAME_SIZE]) { + if (data[0] != FRAME_START_BYTE_1 || data[1] != PASSIVE_START_BYTE_2 || data[22] != FRAME_STOP_BYTE) + return false; + uint8_t sum = 0; + for (size_t i = 0; i < 21; ++i) + sum += data[i]; + return data[21] == (sum & 0xFF); +} + +static void passive_no_id_to_active_frame(const uint8_t passive[PASSIVE_FRAME_SIZE], uint8_t active[FRAME_SIZE]) { + std::memset(active, 0, FRAME_SIZE); + active[0] = FRAME_START_BYTE_1; + active[1] = FRAME_START_BYTE_2; + active[7] = 0x01; + active[8] = passive[2]; + for (size_t i = 0; i < 6; ++i) + active[9 + i] = passive[3 + i]; + active[15] = passive[9]; + for (size_t i = 0; i < 5; ++i) + active[16 + i] = passive[10 + i]; + active[21] = FRAME_FLAG_RESERVED_SECTION; + active[24] = passive[15]; + for (size_t i = 0; i < 3; ++i) + active[25 + i] = passive[16 + i]; + active[28] = passive[19]; + active[29] = passive[20]; + active[30] = passive[21]; + active[31] = FRAME_STOP_BYTE; +} + +static float read_accumulated_flow(const uint8_t data[FRAME_SIZE]) { return (data[FRAME_ACC_FLOW_FLAG_INDEX] == ACC_FLOW_M3_FLAG ? L_PER_M3 : 1.0f) * (to_float(data[14]) * 10000000.0f + to_float(data[13]) * 100000.0f + to_float(data[12]) * 1000.0f + to_float(data[11]) * 10.0f + to_float(data[10]) * 0.1f + to_float(data[9]) * 0.001f); } -static float read_flow(uint8_t data[FRAME_SIZE]) { +static float read_flow(const uint8_t data[FRAME_SIZE]) { return (data[FRAME_FLOW_SIGN_INDEX] == FLOW_NEGATIVE_SIGN ? -1.0f : 1.0f) * (to_float(data[19]) * 10000.0f + to_float(data[18]) * 100.0f + to_float(data[17]) + to_float(data[16]) * 0.01f) * @@ -86,7 +128,7 @@ static void log_hex(const uint8_t *data, size_t len) { ESP_LOGD(TAG, "%s", format_hex_pretty_to(hex_buf, data, len, ' ')); } -static float read_temperature(uint8_t data[FRAME_SIZE]) { +static float read_temperature(const uint8_t data[FRAME_SIZE]) { // happens sometimes before getting a real reading if (data[27] == 0x00 && (data[26] == 0x00 || data[26] == 0x70) && data[25] == 0x00) { return NAN; @@ -106,19 +148,39 @@ static bool read_flow_rate_out_of_range(const uint8_t data[FRAME_SIZE]) { return data[FRAME_ST2_INDEX] & ST2_FLOW_RATE_OUT_OF_RANGE_MASK; } -bool UFM01Component::send_command_(const std::array &command) { +void UFM01Component::flush_rx_() { + while (this->available()) { + uint8_t byte; + this->read_byte(&byte); + } + this->read_index_ = 0; +} + +void UFM01Component::send_command_no_wait_(const std::array &command) { + this->flush_rx_(); this->write_array(command); this->flush(); +} + +// Drains whatever is currently in the RX buffer, looking for a command ACK. +bool UFM01Component::consume_ack_() { + while (this->available()) { + uint8_t byte; + if (!this->read_byte(&byte)) + return false; + if (byte == COMMAND_ACK) + return true; + ESP_LOGV(TAG, "Unexpected byte while waiting for command ACK: 0x%02X", byte); + } + return false; +} + +bool UFM01Component::send_command_(const std::array &command) { + this->send_command_no_wait_(command); const uint32_t start = millis(); while (millis() - start < COMMAND_ACK_TIMEOUT_MS) { - if (this->available()) { - uint8_t byte; - if (this->read_byte(&byte)) { - if (byte == COMMAND_ACK) - return true; - ESP_LOGV(TAG, "Unexpected byte while waiting for command ACK: 0x%02X", byte); - } - } + if (this->consume_ack_()) + return true; delay(1); } return false; @@ -130,14 +192,12 @@ bool UFM01Component::clear_accumulated_flow_() { return this->send_command_(CLEA bool UFM01Component::set_active_mode_() { return this->send_command_(ACTIVE_MODE); } -float UFM01Component::get_setup_priority() const { return setup_priority::IO; } +float UFM01Component::get_setup_priority() const { return setup_priority::LATE; } void UFM01Component::setup() { ESP_LOGI(TAG, "Setting up UFM-01..."); - if (!this->set_active_mode_()) { - ESP_LOGW(TAG, "Failed to set active mode (no ACK from device)"); - this->mark_failed(); - } + this->startup_wait_ms_ = STARTUP_DELAY_MS; + this->set_startup_phase_(StartupPhase::WAIT); } void UFM01Component::dump_config() { @@ -154,12 +214,9 @@ void UFM01Component::dump_config() { LOG_BINARY_SENSOR(" ", "Flow Rate Out Of Range", this->flow_rate_out_of_range_binary_sensor_); #endif this->check_uart_settings(2400, 1, uart::UART_CONFIG_PARITY_EVEN, 8); - if (this->is_failed()) { - ESP_LOGW(TAG, "Setup failed: active mode not acknowledged by device"); - } } -void UFM01Component::on_data_(uint8_t data[FRAME_SIZE]) { +void UFM01Component::on_active_frame_(uint8_t data[FRAME_SIZE]) { bool empty_tube = read_empty_tube(data); #ifdef USE_BINARY_SENSOR if (this->ufc_chip_error_binary_sensor_ != nullptr) @@ -189,10 +246,14 @@ void UFM01Component::on_data_(uint8_t data[FRAME_SIZE]) { this->temperature_sensor_->publish_state(read_temperature(data)); } #endif + this->last_valid_frame_ms_ = millis(); + this->status_clear_warning(); + this->status_clear_error(); } -void UFM01Component::loop() { - // Drain the UART buffer each loop, reading one byte at a time into the frame +bool UFM01Component::process_active_stream_() { + bool got_valid_frame = false; + while (this->available()) { if (!this->read_byte(&this->data_[this->read_index_])) { ESP_LOGW(TAG, "unable to read byte"); @@ -201,23 +262,22 @@ void UFM01Component::loop() { } if ((this->read_index_ == 0 && this->data_[0] != FRAME_START_BYTE_1) || (this->read_index_ == 1 && this->data_[1] != FRAME_START_BYTE_2)) { - ESP_LOGW(TAG, "not start of data at %d (is 0x%02X)", this->read_index_, this->data_[this->read_index_]); + ESP_LOGD(TAG, "not start of data at %d (is 0x%02X)", this->read_index_, this->data_[this->read_index_]); this->read_index_ = 0; continue; } if (++this->read_index_ < static_cast(FRAME_SIZE)) continue; - // Full frame received - if (validate_data(this->data_)) { - this->on_data_(this->data_); + if (validate_active_frame(this->data_)) { + this->on_active_frame_(this->data_); this->read_index_ = 0; + got_valid_frame = true; continue; } - // Invalid frame: try to resync on the next start marker within the buffer log_hex(this->data_, sizeof(this->data_)); - ESP_LOGE(TAG, "unable to read data"); + ESP_LOGW(TAG, "unable to read data"); for (int32_t i = 2; i < static_cast(FRAME_STOP_INDEX) && this->read_index_ == static_cast(FRAME_SIZE); ++i) { if ((this->data_[i] == FRAME_START_BYTE_1) && (this->data_[i + 1] == FRAME_START_BYTE_2)) { @@ -229,6 +289,190 @@ void UFM01Component::loop() { if (this->read_index_ == static_cast(FRAME_SIZE)) this->read_index_ = 0; } + + return got_valid_frame; +} + +void UFM01Component::set_startup_phase_(StartupPhase phase) { + this->startup_phase_ = phase; + this->phase_start_ms_ = millis(); +} + +void UFM01Component::enter_active_stream_(const char *reason) { + ESP_LOGI(TAG, "UFM-01 active stream %s", reason); + this->operating_mode_ = OperatingMode::ACTIVE_STREAM; + this->passive_read_pending_ = false; +} + +void UFM01Component::start_passive_read_() { + this->send_command_no_wait_(READ_SENSOR_DATA_NO_ID); + this->passive_index_ = 0; + this->passive_start_ms_ = millis(); +} + +// Accumulates the reply to a passive read request across loop iterations. +PassiveReadResult UFM01Component::continue_passive_read_() { + while (this->available() && this->passive_index_ < PASSIVE_FRAME_SIZE) { + uint8_t byte; + if (!this->read_byte(&byte)) + break; + + if (this->passive_index_ == 0 && byte != FRAME_START_BYTE_1) + continue; + if (this->passive_index_ == 1 && byte != PASSIVE_START_BYTE_2) { + // The mismatched byte may itself be the start of the real frame + this->passive_index_ = (byte == FRAME_START_BYTE_1) ? 1 : 0; + continue; + } + this->passive_frame_[this->passive_index_++] = byte; + } + + if (this->passive_index_ < PASSIVE_FRAME_SIZE) { + if (millis() - this->passive_start_ms_ < PASSIVE_READ_TIMEOUT_MS) + return PassiveReadResult::PASSIVE_READ_RESULT_PENDING; + ESP_LOGD(TAG, "passive read timeout (%zu/%zu bytes)", this->passive_index_, PASSIVE_FRAME_SIZE); + return PassiveReadResult::PASSIVE_READ_RESULT_FAILURE; + } + + if (!validate_passive_frame(this->passive_frame_)) { + log_hex(this->passive_frame_, PASSIVE_FRAME_SIZE); + ESP_LOGW(TAG, "invalid passive frame"); + return PassiveReadResult::PASSIVE_READ_RESULT_FAILURE; + } + + uint8_t active_frame[FRAME_SIZE]; + passive_no_id_to_active_frame(this->passive_frame_, active_frame); + this->on_active_frame_(active_frame); + return PassiveReadResult::PASSIVE_READ_RESULT_SUCCESS; +} + +void UFM01Component::loop_startup_() { + const uint32_t elapsed = millis() - this->phase_start_ms_; + + switch (this->startup_phase_) { + case StartupPhase::WAIT: + // Pick up an already-streaming device without resetting it + if (this->process_active_stream_()) { + this->enter_active_stream_("started"); + return; + } + if (elapsed < this->startup_wait_ms_) + return; + ESP_LOGD(TAG, "Running startup sequence"); + this->status_set_warning("initializing UFM-01"); + this->reset_retried_ = false; + this->send_command_no_wait_(RESET_DEVICE); + this->set_startup_phase_(StartupPhase::RESET_WAIT_ACK); + return; + + case StartupPhase::RESET_WAIT_ACK: + if (this->consume_ack_()) { + this->set_startup_phase_(StartupPhase::POST_RESET_WAIT); + return; + } + if (elapsed < COMMAND_ACK_TIMEOUT_MS) + return; + if (!this->reset_retried_) { + ESP_LOGW(TAG, "Reset not acknowledged, retrying in %" PRIu32 " ms", RESET_RETRY_DELAY_MS); + this->set_startup_phase_(StartupPhase::RESET_RETRY_WAIT); + } else { + ESP_LOGW(TAG, "Reset failed during startup"); + this->set_startup_phase_(StartupPhase::POST_RESET_WAIT); + } + return; + + case StartupPhase::RESET_RETRY_WAIT: + if (elapsed < RESET_RETRY_DELAY_MS) + return; + this->reset_retried_ = true; + this->send_command_no_wait_(RESET_DEVICE); + this->set_startup_phase_(StartupPhase::RESET_WAIT_ACK); + return; + + case StartupPhase::POST_RESET_WAIT: + if (elapsed < POST_RESET_DELAY_MS) + return; + this->send_command_no_wait_(ACTIVE_MODE); + this->set_startup_phase_(StartupPhase::ACTIVE_WAIT_FRAME); + return; + + case StartupPhase::ACTIVE_WAIT_FRAME: + // The command ACK (0xE5) is consumed by the frame parser as noise + if (this->process_active_stream_()) { + this->enter_active_stream_("started"); + return; + } + if (elapsed < ACTIVE_FRAME_TIMEOUT_MS) + return; + this->start_passive_read_(); + this->set_startup_phase_(StartupPhase::PASSIVE_WAIT_REPLY); + return; + + case StartupPhase::PASSIVE_WAIT_REPLY: + switch (this->continue_passive_read_()) { + case PassiveReadResult::PASSIVE_READ_RESULT_PENDING: + return; + case PassiveReadResult::PASSIVE_READ_RESULT_SUCCESS: + ESP_LOGI(TAG, "UFM-01 using passive polling"); + this->operating_mode_ = OperatingMode::PASSIVE_POLL; + this->passive_read_pending_ = false; + this->last_poll_ms_ = millis(); + return; + case PassiveReadResult::PASSIVE_READ_RESULT_FAILURE: + ESP_LOGW(TAG, "Startup failed, retrying in %" PRIu32 " ms", STARTUP_RETRY_MS); + this->startup_wait_ms_ = STARTUP_RETRY_MS; + this->set_startup_phase_(StartupPhase::WAIT); + return; + } + } +} + +void UFM01Component::loop_active_stream_() { + this->process_active_stream_(); + if (this->last_valid_frame_ms_ != 0 && millis() - this->last_valid_frame_ms_ > ACTIVE_STALE_MS) { + ESP_LOGW(TAG, "Active stream stale, switching to passive polling"); + this->operating_mode_ = OperatingMode::PASSIVE_POLL; + this->passive_read_pending_ = false; + this->last_poll_ms_ = 0; + this->status_set_warning("UFM-01 passive poll"); + } +} + +void UFM01Component::loop_passive_poll_() { + if (this->passive_read_pending_) { + const PassiveReadResult result = this->continue_passive_read_(); + if (result == PassiveReadResult::PASSIVE_READ_RESULT_PENDING) + return; + this->passive_read_pending_ = false; + if (result == PassiveReadResult::PASSIVE_READ_RESULT_FAILURE) + this->status_set_warning("UFM-01 passive poll failed"); + return; + } + + if (this->process_active_stream_()) { + this->enter_active_stream_("resumed"); + return; + } + + if (millis() - this->last_poll_ms_ >= PASSIVE_POLL_INTERVAL_MS) { + this->last_poll_ms_ = millis(); + this->start_passive_read_(); + this->passive_read_pending_ = true; + } +} + +void UFM01Component::loop() { + switch (this->operating_mode_) { + case OperatingMode::STARTUP: + this->loop_startup_(); + return; + case OperatingMode::ACTIVE_STREAM: + this->loop_active_stream_(); + return; + case OperatingMode::PASSIVE_POLL: + this->loop_passive_poll_(); + return; + } } } // namespace esphome::ufm01 diff --git a/esphome/components/ufm01/ufm01.h b/esphome/components/ufm01/ufm01.h index e759de9169..0a39dcc9af 100644 --- a/esphome/components/ufm01/ufm01.h +++ b/esphome/components/ufm01/ufm01.h @@ -11,12 +11,39 @@ #include "esphome/components/uart/uart.h" #include +#include // component API definition at https://www.sciosense.com/wp-content/uploads/2025/06/UFM-01-Datasheet-1.pdf namespace esphome::ufm01 { +namespace testing { +class TestableUFM01; +} // namespace testing + static constexpr size_t FRAME_SIZE = 32; +static constexpr size_t PASSIVE_FRAME_SIZE = 23; + +enum class OperatingMode : uint8_t { + STARTUP = 0, + ACTIVE_STREAM = 1, + PASSIVE_POLL = 2, +}; + +enum class StartupPhase : uint8_t { + WAIT = 0, + RESET_WAIT_ACK = 1, + RESET_RETRY_WAIT = 2, + POST_RESET_WAIT = 3, + ACTIVE_WAIT_FRAME = 4, + PASSIVE_WAIT_REPLY = 5, +}; + +enum class PassiveReadResult : uint8_t { + PASSIVE_READ_RESULT_PENDING = 0, + PASSIVE_READ_RESULT_SUCCESS = 1, + PASSIVE_READ_RESULT_FAILURE = 2, +}; class UFM01Component : public uart::UARTDevice, public Component { #ifdef USE_SENSOR @@ -48,10 +75,37 @@ class UFM01Component : public uart::UARTDevice, public Component { private: bool send_command_(const std::array &command); + void send_command_no_wait_(const std::array &command); + bool consume_ack_(); + void flush_rx_(); + bool process_active_stream_(); + void on_active_frame_(uint8_t data[FRAME_SIZE]); + + void loop_startup_(); + void loop_active_stream_(); + void loop_passive_poll_(); + void set_startup_phase_(StartupPhase phase); + void enter_active_stream_(const char *reason); + void start_passive_read_(); + PassiveReadResult continue_passive_read_(); + + OperatingMode operating_mode_{OperatingMode::STARTUP}; + StartupPhase startup_phase_{StartupPhase::WAIT}; + uint32_t phase_start_ms_{0}; + uint32_t startup_wait_ms_{0}; + bool reset_retried_{false}; + uint32_t last_valid_frame_ms_{0}; + uint32_t last_poll_ms_{0}; + + bool passive_read_pending_{false}; + uint32_t passive_start_ms_{0}; + size_t passive_index_{0}; + uint8_t passive_frame_[PASSIVE_FRAME_SIZE]; int32_t read_index_ = 0; uint8_t data_[FRAME_SIZE]; - void on_data_(uint8_t data[FRAME_SIZE]); + + friend class testing::TestableUFM01; }; } // namespace esphome::ufm01 diff --git a/esphome/components/web_server/__init__.py b/esphome/components/web_server/__init__.py index 2587d13b9e..c1887cc3fc 100644 --- a/esphome/components/web_server/__init__.py +++ b/esphome/components/web_server/__init__.py @@ -1,5 +1,6 @@ from __future__ import annotations +import base64 import gzip import logging import re @@ -406,10 +407,21 @@ async def to_code(config): # The scheme is fixed at build time so the unused Basic/Digest code path is compiled # out. Basic is the current default (the absence of this define); an explicit # 'type: digest' opts in early. Default changes to digest in 2027.1.0. - if auth.get(CONF_TYPE) == AUTH_TYPE_DIGEST: + is_digest = auth.get(CONF_TYPE) == AUTH_TYPE_DIGEST + if is_digest: cg.add_define("USE_WEBSERVER_AUTH_DIGEST") - cg.add(paren.set_auth_username(auth[CONF_USERNAME])) - cg.add(paren.set_auth_password(auth[CONF_PASSWORD])) + if is_digest or CORE.is_esp32: + cg.add(paren.set_auth_username(auth[CONF_USERNAME])) + cg.add(paren.set_auth_password(auth[CONF_PASSWORD])) + else: + # Every non-ESP32 basic auth build takes this path. The ESP8266 and RP2040 + # core base64 encoders wrap output every 72 chars, which breaks + # ESPAsyncWebServer's basic auth compare for long credentials. + # Precompute the hash here and let C++ compare the raw header payload. + basic_hash = base64.b64encode( + f"{auth[CONF_USERNAME]}:{auth[CONF_PASSWORD]}".encode() + ).decode() + cg.add(paren.set_auth_basic_hash(basic_hash)) if CONF_CSS_INCLUDE in config: cg.add_define("USE_WEBSERVER_CSS_INCLUDE") path = CORE.relative_config_path(config[CONF_CSS_INCLUDE]) diff --git a/esphome/components/web_server_base/web_server_base.cpp b/esphome/components/web_server_base/web_server_base.cpp index ccfc04f674..873c5b5a49 100644 --- a/esphome/components/web_server_base/web_server_base.cpp +++ b/esphome/components/web_server_base/web_server_base.cpp @@ -7,7 +7,7 @@ WebServerBase *global_web_server_base = nullptr; // NOLINT(cppcoreguidelines-av void WebServerBase::add_handler(AsyncWebHandler *handler) { #ifdef USE_WEBSERVER_AUTH - if (!credentials_.username.empty()) { + if (credentials_.is_set()) { handler = new internal::AuthMiddlewareHandler(handler, &credentials_); } #endif diff --git a/esphome/components/web_server_base/web_server_base.h b/esphome/components/web_server_base/web_server_base.h index 9657853a73..c647a13b50 100644 --- a/esphome/components/web_server_base/web_server_base.h +++ b/esphome/components/web_server_base/web_server_base.h @@ -1,7 +1,6 @@ #pragma once #include "esphome/core/defines.h" #if defined(USE_NETWORK) && !defined(USE_ZEPHYR) -#include #include #include "esphome/core/progmem.h" @@ -46,9 +45,20 @@ class MiddlewareHandler : public AsyncWebHandler { }; #ifdef USE_WEBSERVER_AUTH +// All fields point to string literals in generated code; nothing is copied. struct Credentials { - std::string username; - std::string password; +#if USE_ESP32 || defined(USE_WEBSERVER_AUTH_DIGEST) + const char *username{nullptr}; + const char *password{nullptr}; + bool is_set() const { return username != nullptr; } +#else + // base64("username:password"), precomputed at codegen time. Used by every non-ESP32 basic + // auth build. The ESP8266 and RP2040 core libb64 wraps base64 output every 72 chars, so + // letting the library encode and compare fails for long credentials; instead the header + // payload is compared against this hash. + const char *basic_auth_hash{nullptr}; + bool is_set() const { return basic_auth_hash != nullptr; } +#endif }; class AuthMiddlewareHandler : public MiddlewareHandler { @@ -57,10 +67,14 @@ class AuthMiddlewareHandler : public MiddlewareHandler { : MiddlewareHandler(next), credentials_(credentials) {} bool check_auth(AsyncWebServerRequest *request) { - bool success = request->authenticate(credentials_->username.c_str(), credentials_->password.c_str()); + // The scheme is chosen at build time (USE_WEBSERVER_AUTH_DIGEST); the unused path is + // compiled out. On ESP32 our own server picks the scheme internally. +#if USE_ESP32 || defined(USE_WEBSERVER_AUTH_DIGEST) + bool success = request->authenticate(credentials_->username, credentials_->password); +#else + bool success = request->authenticate(credentials_->basic_auth_hash); +#endif if (!success) { - // The scheme is chosen at build time (USE_WEBSERVER_AUTH_DIGEST); the unused path is - // compiled out. On ESP32 our own server picks the scheme internally. #if USE_ESP32 request->requestAuthentication(); #elif defined(USE_WEBSERVER_AUTH_DIGEST) @@ -125,8 +139,12 @@ class WebServerBase final { AsyncWebServer *get_server() const { return this->server_; } #ifdef USE_WEBSERVER_AUTH - void set_auth_username(std::string auth_username) { credentials_.username = std::move(auth_username); } - void set_auth_password(std::string auth_password) { credentials_.password = std::move(auth_password); } +#if USE_ESP32 || defined(USE_WEBSERVER_AUTH_DIGEST) + void set_auth_username(const char *auth_username) { credentials_.username = auth_username; } + void set_auth_password(const char *auth_password) { credentials_.password = auth_password; } +#else + void set_auth_basic_hash(const char *hash) { credentials_.basic_auth_hash = hash; } +#endif #endif void add_handler(AsyncWebHandler *handler); diff --git a/esphome/components/wifi/wifi_component.cpp b/esphome/components/wifi/wifi_component.cpp index 182e86daed..9e78e7c48e 100644 --- a/esphome/components/wifi/wifi_component.cpp +++ b/esphome/components/wifi/wifi_component.cpp @@ -1676,7 +1676,7 @@ void WiFiComponent::check_connecting_finished(uint32_t now) { this->clear_all_bssid_priorities_(); #ifdef USE_WIFI_FAST_CONNECT - this->save_fast_connect_settings_(); + this->save_fast_connect_settings_(this->wifi_bssid(), get_wifi_channel()); #endif this->release_scan_results_(); @@ -2301,9 +2301,7 @@ bool WiFiComponent::load_fast_connect_settings_(WiFiAP ¶ms) { return false; } -void WiFiComponent::save_fast_connect_settings_() { - bssid_t bssid = wifi_bssid(); - uint8_t channel = get_wifi_channel(); +void WiFiComponent::save_fast_connect_settings_(const bssid_t &bssid, uint8_t channel) { // selected_sta_index_ is always valid here (called only after successful connection) // Fallback to 0 is defensive programming for robustness int8_t ap_index = this->selected_sta_index_ >= 0 ? this->selected_sta_index_ : 0; @@ -2416,6 +2414,25 @@ void WiFiComponent::clear_roaming_state_() { this->roaming_state_ = RoamingState::IDLE; } +#ifdef USE_ESP32 +void WiFiComponent::handle_driver_roam_(const bssid_t &bssid, uint8_t channel) { + // A driver-initiated roam (e.g. 802.11v BTM) re-associates without the state + // machine ever leaving STA_CONNECTED, so check_connecting_finished() never runs. + // Redo its post-connect bookkeeping here. roaming_state_ is deliberately left + // untouched so an in-flight roaming scan is not orphaned. The BSSID and + // channel both come from the connected event so the saved pair is consistent: + // the radio may be off-channel during a roaming scan, and a later queued + // event may have moved the driver on again by the time this one is processed. + this->roaming_last_check_ = App.get_loop_component_start_time(); + this->roaming_attempts_ = 0; + this->roaming_scan_end_ = 0; + this->clear_all_bssid_priorities_(); +#ifdef USE_WIFI_FAST_CONNECT + this->save_fast_connect_settings_(bssid, channel); +#endif +} +#endif + void WiFiComponent::release_scan_results_() { if (!this->keep_scan_results_) { ScanResultsLock lock(this); diff --git a/esphome/components/wifi/wifi_component.h b/esphome/components/wifi/wifi_component.h index 43e44a135f..a851ea4015 100644 --- a/esphome/components/wifi/wifi_component.h +++ b/esphome/components/wifi/wifi_component.h @@ -781,13 +781,19 @@ class WiFiComponent final : public Component { #ifdef USE_WIFI_FAST_CONNECT bool load_fast_connect_settings_(WiFiAP ¶ms); - void save_fast_connect_settings_(); + void save_fast_connect_settings_(const bssid_t &bssid, uint8_t channel); #endif // Post-connect roaming methods void check_roaming_(uint32_t now); void process_roaming_scan_(); void clear_roaming_state_(); +#ifdef USE_ESP32 + /// Redo post-connect bookkeeping after a driver-initiated roam (e.g. 802.11v BTM) + /// @param bssid The new AP's BSSID, taken from the connected event + /// @param channel The new AP's channel, taken from the connected event + void handle_driver_roam_(const bssid_t &bssid, uint8_t channel); +#endif /// Returns true if a component has requested that roaming scans be suppressed (e.g. during audio playback). bool roaming_suppressed_() const { diff --git a/esphome/components/wifi/wifi_component_esp_idf.cpp b/esphome/components/wifi/wifi_component_esp_idf.cpp index d78cd21380..783c000f7b 100644 --- a/esphome/components/wifi/wifi_component_esp_idf.cpp +++ b/esphome/components/wifi/wifi_component_esp_idf.cpp @@ -825,6 +825,19 @@ void WiFiComponent::wifi_process_event_(IDFWiFiEvent *data) { (const char *) it.ssid, bssid_buf, it.channel, get_auth_mode_str(it.authmode)); #endif s_sta_connected = true; + if (this->state_ == WIFI_COMPONENT_STATE_STA_CONNECTED) { + // Driver-initiated roam: the WIFI_REASON_ROAMING disconnect was ignored, + // so the state machine never left STA_CONNECTED. +#if ESPHOME_LOG_LEVEL >= ESPHOME_LOG_LEVEL_INFO + char roam_bssid_s[MAC_ADDRESS_PRETTY_BUFFER_SIZE]; + format_mac_addr_upper(it.bssid, roam_bssid_s); + ESP_LOGI(TAG, "Roamed ssid='%.*s' bssid=" LOG_SECRET("%s") " channel=%u", it.ssid_len, (const char *) it.ssid, + roam_bssid_s, it.channel); +#endif + bssid_t roam_bssid; + std::copy(it.bssid, it.bssid + 6, roam_bssid.begin()); + this->handle_driver_roam_(roam_bssid, it.channel); + } #ifdef USE_WIFI_CONNECT_STATE_LISTENERS // Defer listener notification until state machine reaches STA_CONNECTED // This ensures wifi.connected condition returns true in listener automations diff --git a/esphome/config_validation.py b/esphome/config_validation.py index c04d43bbee..0eebf12e66 100644 --- a/esphome/config_validation.py +++ b/esphome/config_validation.py @@ -2331,13 +2331,13 @@ def _validate_no_slash(value): the visually similar Unicode FRACTION SLASH (U+2044) character. """ if "/" in value: - # Remove before 2026.7.0 + # Remove before 2027.7.0 new_value = value.replace("/", FRACTION_SLASH) _LOGGER.warning( "'%s' contains '/' which is reserved as a URL path separator. " "Automatically replacing with '%s' (Unicode FRACTION SLASH). " "Please update your configuration. " - "This will become an error in ESPHome 2026.7.0.", + "This will become an error in ESPHome 2027.7.0.", value, new_value, ) diff --git a/esphome/core/defines.h b/esphome/core/defines.h index fdcfa2aa8a..21cea31749 100644 --- a/esphome/core/defines.h +++ b/esphome/core/defines.h @@ -157,9 +157,17 @@ #define USE_OUTPUT_FLOAT_POWER_SCALING #define USE_POWER_SUPPLY #define USE_PREFERENCES_SYNC_EVERY_LOOP -// Only defined by key-lookup preference backends (esp32, libretiny, host, zephyr); -// slot-based platforms (esp8266, rp2040) never set it in generated builds +// Only defined by key-lookup preference backends; the slot-based platforms +// (esp8266, rp2040) never set it in generated builds, and their preferences +// managers do not provide load_from_key(), so the PreferencesKeyLookupContract +// assert would fail their clang-tidy environments. Written as a deny-list so +// the no-platform analysis configuration (whose Preferences stub provides +// load_from_key()) keeps covering the key-lookup code paths, and so a future +// slot-based platform fails the assert loudly instead of silently losing +// analysis coverage. +#if !defined(USE_ESP8266) && !defined(USE_RP2) #define USE_PREFERENCE_KEY_LOOKUP +#endif #define USE_PROVISIONING #define USE_QR_CODE #define USE_SAFE_MODE_BOOT_IS_GOOD_ON_SHUTDOWN @@ -389,6 +397,7 @@ #define USE_ETHERNET_W6100 #define USE_ETHERNET_W6300 #define USE_ETHERNET_DM9051 +#define USE_ETHERNET_CH390 #define CONFIG_ETH_SPI_ETHERNET_W5500 1 #define CONFIG_ETH_SPI_ETHERNET_DM9051 1 #define CONFIG_ETH_USE_ESP32_EMAC 1 diff --git a/esphome/core/preference_backend.h b/esphome/core/preference_backend.h index b9bb9a0252..0622376fca 100644 --- a/esphome/core/preference_backend.h +++ b/esphome/core/preference_backend.h @@ -1,5 +1,6 @@ #pragma once +#include #include #include "esphome/core/defines.h" @@ -30,6 +31,15 @@ namespace esphome { +// The PreferenceBackend method surface, asserted on the alias each platform +// header binds. save() persists len bytes; load() fills dest only when the +// stored data exists and matches len. Both report success as their return. +template +concept PreferenceBackendContract = requires(T backend, const uint8_t *src, uint8_t *dest, size_t len) { + { backend.save(src, len) } -> std::same_as; + { backend.load(dest, len) } -> std::same_as; +}; + #if !defined(USE_ESP32) && !defined(USE_ESP8266) && !defined(USE_RP2) && !defined(USE_LIBRETINY) && \ !defined(USE_HOST) && !(defined(USE_ZEPHYR) && defined(CONFIG_SETTINGS)) // Stub for static analysis when no platform is defined. @@ -40,6 +50,8 @@ struct PreferenceBackend { #endif using ESPPreferenceBackend = PreferenceBackend; +static_assert(PreferenceBackendContract, + "The platform's preference backend is missing part of the PreferenceBackend surface"); class ESPPreferenceObject { public: @@ -68,6 +80,39 @@ class ESPPreferenceObject { PreferenceBackend *backend_{nullptr}; }; +// The preferences manager method surface, asserted in esphome/core/preferences.h +// on the ESPPreferences alias each platform's preferences.h binds through +// DECLARE_PREFERENCE_ALIASES. Semantics beyond the signatures: +// - make_preference: the two-argument form applies the platform's historic +// default storage; in_flash=false may fall back to flash where the platform +// has no faster storage. +// - sync: commit pending writes to flash, true on success. +// - reset: forget unsaved changes and re-initialize the permanent storage +// (usually followed by a restart), true on success. +// The template forms are what component call sites use; PreferencesMixin +// supplies them, but the derived class's non-template overloads hide them +// unless it also declares `using PreferencesMixin::make_preference;`, so +// the concept pins those too. +template +concept PreferencesContract = requires(T prefs, size_t len, uint32_t type, bool in_flash) { + { prefs.make_preference(len, type, in_flash) } -> std::same_as; + { prefs.make_preference(len, type) } -> std::same_as; + { prefs.template make_preference(type, in_flash) } -> std::same_as; + { prefs.template make_preference(type) } -> std::same_as; + { prefs.sync() } -> std::same_as; + { prefs.reset() } -> std::same_as; +}; + +// Key-lookup platforms additionally provide load_from_key(), a one-shot read +// of a stored preference by key that migrate_preference() relies on; see the +// key-lookup note at the top of this file. Not part of PreferencesContract, +// so it is asserted in preferences.h only where USE_PREFERENCE_KEY_LOOKUP +// is set. +template +concept PreferencesKeyLookupContract = requires(T prefs, uint32_t type, uint8_t *data, size_t len) { + { prefs.load_from_key(type, data, len) } -> std::same_as; +}; + /// CRTP mixin providing type-safe template make_preference() helpers. /// Platform preferences classes inherit this to avoid duplicating these templates. template class PreferencesMixin { diff --git a/esphome/core/preferences.h b/esphome/core/preferences.h index d24d51164a..cfeddebda7 100644 --- a/esphome/core/preferences.h +++ b/esphome/core/preferences.h @@ -45,8 +45,18 @@ extern ESPPreferences *global_preferences; // NOLINT(cppcoreguidelines-avoid-no } // namespace esphome #endif +namespace esphome { +static_assert(PreferencesContract, + "The platform's preferences manager is missing part of the ESPPreferences surface " + "(esphome/core/preference_backend.h)"); +} // namespace esphome + #ifdef USE_PREFERENCE_KEY_LOOKUP namespace esphome { +static_assert(PreferencesKeyLookupContract, + "This platform emits USE_PREFERENCE_KEY_LOOKUP but its preferences manager does not provide " + "load_from_key() (esphome/core/preference_backend.h)"); + /// Copy preference data stored under old_key into new_pref (created for new_key) if the keys /// differ and new_pref has no data yet. scratch must hold at least size bytes. /// Returns true when scratch holds the entity's current data (loaded or just migrated). diff --git a/requirements.txt b/requirements.txt index 4501b733a6..98c2f47d7e 100644 --- a/requirements.txt +++ b/requirements.txt @@ -13,7 +13,7 @@ platformio==6.1.19 esptool==5.3.1 click==8.3.3 aioesphomeapi==45.7.0 -aiohappyeyeballs==2.6.2 # Happy Eyeballs for requests downloads; already pulled in by aioesphomeapi +aiohappyeyeballs==2.7.1 # Happy Eyeballs for requests downloads; already pulled in by aioesphomeapi zeroconf==0.150.0 puremagic==2.2.0 ruamel.yaml==0.19.1 # dashboard_import @@ -34,4 +34,4 @@ filelock==3.32.2 # inter-process locks (PlatformIO cache heal, git clone cache) pyparsing >= 3.3.2 # For autocompletion -argcomplete>=3.7.0 +argcomplete>=3.7.2 diff --git a/requirements_test.txt b/requirements_test.txt index b5753066ba..0905fe6be1 100644 --- a/requirements_test.txt +++ b/requirements_test.txt @@ -1,6 +1,6 @@ pylint==4.0.6 flake8==7.3.0 # also change in .pre-commit-config.yaml when updating -ruff==0.16.1 # also change in .pre-commit-config.yaml when updating +ruff==0.16.2 # also change in .pre-commit-config.yaml when updating pyupgrade==3.21.2 # also change in .pre-commit-config.yaml when updating prek==0.4.12 # also change in .github/workflows/ci.yml when updating diff --git a/tests/component_tests/bk72xx_ble_tracker/config/test_automations.yaml b/tests/component_tests/bk72xx_ble_tracker/config/test_automations.yaml index 994855b782..123d4296db 100644 --- a/tests/component_tests/bk72xx_ble_tracker/config/test_automations.yaml +++ b/tests/component_tests/bk72xx_ble_tracker/config/test_automations.yaml @@ -15,6 +15,7 @@ bk72xx: bk72xx_ble_tracker: scan_parameters: continuous: false + active: false on_ble_advertise: - mac_address: - AC:37:43:77:5F:4C diff --git a/tests/component_tests/bk72xx_ble_tracker/test_automations_codegen.py b/tests/component_tests/bk72xx_ble_tracker/test_automations_codegen.py index 3a03f98adf..777ae76b4f 100644 --- a/tests/component_tests/bk72xx_ble_tracker/test_automations_codegen.py +++ b/tests/component_tests/bk72xx_ble_tracker/test_automations_codegen.py @@ -48,6 +48,8 @@ def test_trigger_codegen( # scan_parameters continuous: false reaches the YAML-mode setter, not the # runtime override. assert "->set_configured_continuous(false)" in main_cpp + # active: false (non-default) flows through to the setter. + assert "->set_scan_active(false)" in main_cpp # Constructor call, not just the declaration: the parent argument is what # registers the trigger as a listener. assert re.search( diff --git a/tests/component_tests/ble_device_base/test_scan_parameter_validation.py b/tests/component_tests/ble_device_base/test_scan_parameter_validation.py index bd41a9476a..2549125a43 100644 --- a/tests/component_tests/ble_device_base/test_scan_parameter_validation.py +++ b/tests/component_tests/ble_device_base/test_scan_parameter_validation.py @@ -16,8 +16,8 @@ from esphome.components.ln882h_ble_tracker import ( from esphome.components.rp2_ble_tracker import SCAN_PARAMETERS_SCHEMA as RP2_SCHEMA -def _validate(**kwargs: str) -> dict: - """Run a scan_parameters config through a passive tracker's real schema.""" +def _validate(**kwargs: str | bool) -> dict: + """Run a scan_parameters config through the bk72xx tracker's real schema.""" return BK72XX_SCHEMA(kwargs) @@ -48,11 +48,12 @@ def test_to_ble_units_truncates() -> None: def test_bk72xx_defaults_are_valid() -> None: - """bk72xx pins the BK reference rate: 100 ms interval, shared 30 ms window.""" + """bk72xx pins the BK reference rate — 100 ms interval, shared 30 ms window — + and exposes active (default on, like every active-capable tracker).""" config = _validate() assert to_ble_units(config["interval"]) == 160 assert to_ble_units(config["window"]) == 48 - assert "active" not in config + assert config["active"] is True def test_esp32_defaults_are_valid() -> None: @@ -86,10 +87,9 @@ def test_esp32_active_can_disable() -> None: assert config["active"] is False -def test_passive_schema_rejects_active_key() -> None: - """Trackers without active scan support must not silently accept the option.""" - with pytest.raises(cv.Invalid): - _validate(active="true") +def test_bk72xx_active_can_disable() -> None: + config = _validate(active=False) + assert config["active"] is False # --- accepted configurations --- diff --git a/tests/component_tests/bluetooth_proxy/test_platform_gates.py b/tests/component_tests/bluetooth_proxy/test_platform_gates.py index a9e33496c3..8fc7ffd23b 100644 --- a/tests/component_tests/bluetooth_proxy/test_platform_gates.py +++ b/tests/component_tests/bluetooth_proxy/test_platform_gates.py @@ -15,6 +15,7 @@ from esphome.const import ( KEY_CORE, KEY_TARGET_FRAMEWORK, KEY_TARGET_PLATFORM, + PLATFORM_BK72XX, PLATFORM_ESP32, PLATFORM_LN882X, PLATFORM_RP2, @@ -27,18 +28,20 @@ from ..types import SetCoreConfigCallable # Advertisement-only hub platforms; rp2 runs the full proxy and has its own # tests below. HUB_PLATFORM_FRAMEWORKS = [ + PlatformFramework.BK72XX_ARDUINO, PlatformFramework.LN882X_ARDUINO, ] HUB_TRACKERS = { + PLATFORM_BK72XX: "bk72xx_ble_tracker", PLATFORM_LN882X: "ln882h_ble_tracker", PLATFORM_RP2: "rp2_ble_tracker", } def test_hub_platform_list_covers_every_hub_platform() -> None: - # A platform added to _HUB_PLATFORMS (bk72xx is planned) would otherwise - # get no gate coverage at all; GATT platforms have their own tests. + # A platform added to _HUB_PLATFORMS would otherwise get no gate coverage + # at all; GATT platforms have their own tests. advertisement_only = set(bluetooth_proxy._HUB_PLATFORMS) - set( bluetooth_connection.HUB_MAX_CONNECTIONS ) diff --git a/tests/component_tests/ethernet/test_ethernet.py b/tests/component_tests/ethernet/test_ethernet.py index b3d37561c7..9308d0b099 100644 --- a/tests/component_tests/ethernet/test_ethernet.py +++ b/tests/component_tests/ethernet/test_ethernet.py @@ -1,13 +1,31 @@ -"""Tests for the ethernet final-validation coexistence gate.""" +"""Tests for the ethernet final-validation coexistence gate and schema bounds.""" import pytest from voluptuous import Invalid -from esphome.components.ethernet import _final_validate +from esphome import config_validation as cv +from esphome.components.esp32 import ( + KEY_BOARD, + KEY_IDF_VERSION, + KEY_VARIANT, + VARIANT_ESP32S3, +) +from esphome.components.ethernet import CONF_CLOCK_SPEED, CONFIG_SCHEMA, _final_validate from esphome.components.network import _validate_priority_list -from esphome.const import CONF_PRIORITY +from esphome.const import CONF_PRIORITY, PlatformFramework +from esphome.core import CORE import esphome.final_validate as fv +from ..types import SetCoreConfigCallable + +_CH390_CONFIG = { + "type": "CH390", + "clk_pin": 47, + "mosi_pin": 48, + "miso_pin": 14, + "cs_pin": 21, +} + @pytest.fixture(autouse=True) def _reset_full_config(): @@ -35,3 +53,40 @@ def test_rejects_wifi_and_ethernet_with_incomplete_priority() -> None: ) with pytest.raises(Invalid, match=r"must.*list both interfaces; missing: wifi"): _final_validate({}) + + +@pytest.mark.parametrize("clock_speed", ["26.67MHz", "72MHz"]) +def test_ch390_accepts_clock_speed_up_to_the_datasheet_maximum( + set_core_config: SetCoreConfigCallable, clock_speed: str +) -> None: + """CH390 SCK is rated to 72MHz, so the schema must accept the whole range.""" + set_core_config( + PlatformFramework.ESP32_IDF, + platform_data={ + KEY_BOARD: "esp32-s3-devkitc-1", + KEY_VARIANT: VARIANT_ESP32S3, + KEY_IDF_VERSION: cv.Version(5, 3, 2), + }, + ) + # _validate derives use_address from the node name, which has no default here. + CORE.name = "ch390-test" + config = CONFIG_SCHEMA({**_CH390_CONFIG, CONF_CLOCK_SPEED: clock_speed}) + assert config[CONF_CLOCK_SPEED] == cv.frequency(clock_speed) + + +def test_ch390_rejects_clock_speed_above_the_datasheet_maximum( + set_core_config: SetCoreConfigCallable, +) -> None: + """The shared 80MHz ceiling is out of spec for this part.""" + set_core_config( + PlatformFramework.ESP32_IDF, + platform_data={ + KEY_BOARD: "esp32-s3-devkitc-1", + KEY_VARIANT: VARIANT_ESP32S3, + KEY_IDF_VERSION: cv.Version(5, 3, 2), + }, + ) + # _validate derives use_address from the node name, which has no default here. + CORE.name = "ch390-test" + with pytest.raises(Invalid, match="value must be at most 72000000"): + CONFIG_SCHEMA({**_CH390_CONFIG, CONF_CLOCK_SPEED: "80MHz"}) diff --git a/tests/component_tests/mitsubishi_cn105/fixtures/top_level_hub_with_legacy_climate_keys.yaml b/tests/component_tests/mitsubishi_cn105/fixtures/top_level_hub_with_legacy_climate_keys.yaml new file mode 100644 index 0000000000..0226d680a4 --- /dev/null +++ b/tests/component_tests/mitsubishi_cn105/fixtures/top_level_hub_with_legacy_climate_keys.yaml @@ -0,0 +1,10 @@ +mitsubishi_cn105: + id: ac_hub + +climate: + - platform: mitsubishi_cn105 + mitsubishi_cn105_id: ac_hub + name: AC + current_temperature_min_interval: 30s + uart_id: uart_bus + update_interval: 10s diff --git a/tests/component_tests/mitsubishi_cn105/test_climate.py b/tests/component_tests/mitsubishi_cn105/test_climate.py new file mode 100644 index 0000000000..e4e3da9c7f --- /dev/null +++ b/tests/component_tests/mitsubishi_cn105/test_climate.py @@ -0,0 +1,30 @@ +"""Tests for Mitsubishi CN105 climate configuration migration diagnostics.""" + +from collections.abc import Callable +from pathlib import Path + +import pytest + +from esphome.components.mitsubishi_cn105 import climate +import esphome.config_validation as cv +from esphome.core import CORE +from esphome.yaml_util import load_yaml + + +def test_top_level_hub_rejects_leftover_legacy_climate_keys( + component_fixture_path: Callable[[str], Path], +) -> None: + config = load_yaml( + component_fixture_path("top_level_hub_with_legacy_climate_keys.yaml") + ) + CORE.raw_config = config + + with pytest.raises(cv.Invalid) as exc_info: + climate.CONFIG_SCHEMA(config["climate"][0]) + + message = str(exc_info.value) + assert "'current_temperature_min_interval'" in message + assert "'uart_id'" in message + assert "'update_interval'" in message + assert "top-level 'mitsubishi_cn105:' block" in message + assert "'telemetry_request_min_interval'" in message diff --git a/tests/component_tests/preferences/__init__.py b/tests/component_tests/preferences/__init__.py new file mode 100644 index 0000000000..e69de29bb2 diff --git a/tests/component_tests/preferences/config/bk72xx.yaml b/tests/component_tests/preferences/config/bk72xx.yaml new file mode 100644 index 0000000000..9ea4154bca --- /dev/null +++ b/tests/component_tests/preferences/config/bk72xx.yaml @@ -0,0 +1,5 @@ +esphome: + name: preftest + +bk72xx: + board: generic-bk7252 diff --git a/tests/component_tests/preferences/config/esp32.yaml b/tests/component_tests/preferences/config/esp32.yaml new file mode 100644 index 0000000000..586979d7b6 --- /dev/null +++ b/tests/component_tests/preferences/config/esp32.yaml @@ -0,0 +1,5 @@ +esphome: + name: preftest + +esp32: + board: esp32dev diff --git a/tests/component_tests/preferences/config/esp8266.yaml b/tests/component_tests/preferences/config/esp8266.yaml new file mode 100644 index 0000000000..b8a1035159 --- /dev/null +++ b/tests/component_tests/preferences/config/esp8266.yaml @@ -0,0 +1,5 @@ +esphome: + name: preftest + +esp8266: + board: esp01_1m diff --git a/tests/component_tests/preferences/config/host.yaml b/tests/component_tests/preferences/config/host.yaml new file mode 100644 index 0000000000..047f8693a8 --- /dev/null +++ b/tests/component_tests/preferences/config/host.yaml @@ -0,0 +1,4 @@ +esphome: + name: preftest + +host: diff --git a/tests/component_tests/preferences/config/nrf52.yaml b/tests/component_tests/preferences/config/nrf52.yaml new file mode 100644 index 0000000000..00892addb5 --- /dev/null +++ b/tests/component_tests/preferences/config/nrf52.yaml @@ -0,0 +1,6 @@ +esphome: + name: preftest + +nrf52: + board: adafruit_itsybitsy_nrf52840 + bootloader: adafruit_nrf52_sd140_v6 diff --git a/tests/component_tests/preferences/config/rp2.yaml b/tests/component_tests/preferences/config/rp2.yaml new file mode 100644 index 0000000000..d57b96a54e --- /dev/null +++ b/tests/component_tests/preferences/config/rp2.yaml @@ -0,0 +1,5 @@ +esphome: + name: preftest + +rp2: + board: rpipicow diff --git a/tests/component_tests/preferences/test_key_lookup_gate.py b/tests/component_tests/preferences/test_key_lookup_gate.py new file mode 100644 index 0000000000..7726113aab --- /dev/null +++ b/tests/component_tests/preferences/test_key_lookup_gate.py @@ -0,0 +1,39 @@ +"""Every preferences platform either emits USE_PREFERENCE_KEY_LOOKUP from +codegen (key-lookup backends) or must not (slot-based backends, whose managers +have no load_from_key()). Run each platform's real codegen and assert the +emission, mirroring the split the deny-list in esphome/core/defines.h assumes +for static analysis. + +The fixtures cover every distinct preferences backend today: ln882x and +rtl87xx route through libretiny (bk72xx stands in for the family), rp2040 is +an alias of rp2, and nrf52 exercises zephyr. A seventh backend needs a new +fixture here.""" + +from collections.abc import Callable +from pathlib import Path + +import pytest + +from esphome.core import CORE + + +@pytest.mark.parametrize( + ("fixture", "emits"), + [ + ("esp32.yaml", True), + ("bk72xx.yaml", True), # libretiny + ("host.yaml", True), + ("nrf52.yaml", True), # zephyr + ("esp8266.yaml", False), + ("rp2.yaml", False), + ], +) +def test_key_lookup_define_matches_the_platform_backend( + fixture: str, + emits: bool, + generate_main: Callable[[str | Path], str], + component_config_path: Callable[[str], Path], +) -> None: + generate_main(component_config_path(fixture)) + defines = {define.name for define in CORE.defines} + assert ("USE_PREFERENCE_KEY_LOOKUP" in defines) is emits diff --git a/tests/component_tests/web_server/test_web_server_auth.py b/tests/component_tests/web_server/test_web_server_auth.py index 82635b26da..183c586a36 100644 --- a/tests/component_tests/web_server/test_web_server_auth.py +++ b/tests/component_tests/web_server/test_web_server_auth.py @@ -33,14 +33,49 @@ def test_web_server_auth_explicit_basic_no_warning( generate_main: Callable[[str], str], caplog: pytest.LogCaptureFixture, ) -> None: - """Auth type basic builds Basic and does not warn.""" - generate_main("tests/component_tests/web_server/web_server_auth_basic.yaml") + """Auth type basic on ESP32 uses plaintext credentials and does not warn.""" + main_cpp = generate_main( + "tests/component_tests/web_server/web_server_auth_basic.yaml" + ) + assert '->set_auth_username("admin");' in main_cpp + assert '->set_auth_password("password");' in main_cpp + assert "set_auth_basic_hash" not in main_cpp assert _has_define("USE_WEBSERVER_AUTH") assert not _has_define("USE_WEBSERVER_AUTH_DIGEST") assert _DEFAULT_CHANGE_WARNING not in caplog.text +def test_web_server_auth_basic_esp8266_uses_precomputed_hash( + generate_main: Callable[[str], str], +) -> None: + """Auth type basic on ESP8266 emits the precomputed base64 hash, not the credentials.""" + main_cpp = generate_main( + "tests/component_tests/web_server/web_server_auth_basic_esp8266.yaml" + ) + + assert '->set_auth_basic_hash("YWRtaW46cGFzc3dvcmQ=");' in main_cpp + assert "set_auth_username" not in main_cpp + assert "set_auth_password" not in main_cpp + assert _has_define("USE_WEBSERVER_AUTH") + assert not _has_define("USE_WEBSERVER_AUTH_DIGEST") + + +def test_web_server_auth_digest_esp8266_uses_plaintext_credentials( + generate_main: Callable[[str], str], +) -> None: + """Auth type digest on ESP8266 uses plaintext credentials, not the basic hash.""" + main_cpp = generate_main( + "tests/component_tests/web_server/web_server_auth_digest_esp8266.yaml" + ) + + assert '->set_auth_username("admin");' in main_cpp + assert '->set_auth_password("password");' in main_cpp + assert "set_auth_basic_hash" not in main_cpp + assert _has_define("USE_WEBSERVER_AUTH") + assert _has_define("USE_WEBSERVER_AUTH_DIGEST") + + def test_web_server_auth_explicit_digest( generate_main: Callable[[str], str], caplog: pytest.LogCaptureFixture, diff --git a/tests/component_tests/web_server/web_server_auth_basic_esp8266.yaml b/tests/component_tests/web_server/web_server_auth_basic_esp8266.yaml new file mode 100644 index 0000000000..79e0c0ccf5 --- /dev/null +++ b/tests/component_tests/web_server/web_server_auth_basic_esp8266.yaml @@ -0,0 +1,16 @@ +--- +esphome: + name: test + +esp8266: + board: esp01_1m + +wifi: + ssid: MySSID + password: password1 + +web_server: + auth: + username: admin + password: password + type: basic diff --git a/tests/component_tests/web_server/web_server_auth_digest_esp8266.yaml b/tests/component_tests/web_server/web_server_auth_digest_esp8266.yaml new file mode 100644 index 0000000000..59565f8733 --- /dev/null +++ b/tests/component_tests/web_server/web_server_auth_digest_esp8266.yaml @@ -0,0 +1,16 @@ +--- +esphome: + name: test + +esp8266: + board: esp01_1m + +wifi: + ssid: MySSID + password: password1 + +web_server: + auth: + username: admin + password: password + type: digest diff --git a/tests/components/bk72xx_ble_tracker/validate-passive.bk72xx-ard.yaml b/tests/components/bk72xx_ble_tracker/validate-passive.bk72xx-ard.yaml new file mode 100644 index 0000000000..ad25fd6b64 --- /dev/null +++ b/tests/components/bk72xx_ble_tracker/validate-passive.bk72xx-ard.yaml @@ -0,0 +1,8 @@ +# Passive scanning variant: the package merge keeps the shared parameters from +# common.yaml and overrides only the mode. +packages: + bk72xx_ble_tracker: !include common.yaml + +bk72xx_ble_tracker: + scan_parameters: + active: false diff --git a/tests/components/ble_device_base/test_scan_response_merger.cpp b/tests/components/ble_device_base/test_scan_response_merger.cpp index 013c7bf8f9..ec157715fd 100644 --- a/tests/components/ble_device_base/test_scan_response_merger.cpp +++ b/tests/components/ble_device_base/test_scan_response_merger.cpp @@ -179,5 +179,15 @@ TEST_F(ScanResponseMergerTest, UnboundMergerDropsInsteadOfCrashing) { EXPECT_TRUE(unbound.empty()); } +TEST_F(ScanResponseMergerTest, PartialBindIsTreatedAsUnbound) { + ScanResponseMerger partial; + partial.bind(&this->dispatcher_, nullptr, "test"); + std::vector data(20, 0xAA); + partial.submit_scan_rsp(MAC_A, -70, 0, data.data(), data.size()); + partial.stash_adv(MAC_A, -40, 0, data.data(), data.size(), 0); + partial.flush(); // dropped, not dispatched through half a binding + EXPECT_TRUE(this->raw_.frames.empty()); +} + } // namespace } // namespace esphome::ble_device_base::testing diff --git a/tests/components/bluetooth_proxy/validate.bk72xx-ard.yaml b/tests/components/bluetooth_proxy/validate.bk72xx-ard.yaml new file mode 100644 index 0000000000..331d679510 --- /dev/null +++ b/tests/components/bluetooth_proxy/validate.bk72xx-ard.yaml @@ -0,0 +1,11 @@ +# Advertisement-only proxy on the bk72xx BLE hub (active-scan-capable since the +# tracker's packed-command start). Config-only: the CI base board generic-bk7252 +# is BLE 4.2 and cannot compile the BLE 5.x tracker. Same bare-hub arrangement +# as test.ln882x-ard.yaml: no explicit ble_hub_id so a grouped build cannot +# collide with bk72xx_ble_tracker's own fixture id. +packages: + common: !include common.yaml + +bk72xx_ble_tracker: + +bluetooth_proxy: diff --git a/tests/components/core/test_preference_contract.cpp b/tests/components/core/test_preference_contract.cpp new file mode 100644 index 0000000000..f0833a5929 --- /dev/null +++ b/tests/components/core/test_preference_contract.cpp @@ -0,0 +1,91 @@ +// Pins the preferences contract concepts so the surface they enforce cannot +// drift unnoticed: a minimal conforming type must satisfy each concept, and a +// type missing a method or returning the wrong type must not. + +#include + +#include "esphome/core/preference_backend.h" + +namespace esphome::core::testing { + +struct MinimalBackend { + bool save(const uint8_t *, size_t) { return true; } + bool load(uint8_t *, size_t) { return true; } +}; +static_assert(PreferenceBackendContract); + +struct BackendMissingLoad { + bool save(const uint8_t *, size_t) { return true; } +}; +static_assert(!PreferenceBackendContract); + +struct BackendWrongReturn { + void save(const uint8_t *, size_t) {} + bool load(uint8_t *, size_t) { return true; } +}; +static_assert(!PreferenceBackendContract); + +struct MinimalPreferences : public PreferencesMixin { + using PreferencesMixin::make_preference; + ESPPreferenceObject make_preference(size_t, uint32_t, bool) { return {}; } + ESPPreferenceObject make_preference(size_t, uint32_t) { return {}; } + bool sync() { return true; } + bool reset() { return true; } +}; +static_assert(PreferencesContract); + +struct PreferencesMissingTwoArgForm : public PreferencesMixin { + using PreferencesMixin::make_preference; + ESPPreferenceObject make_preference(size_t, uint32_t, bool) { return {}; } + bool sync() { return true; } + bool reset() { return true; } +}; +static_assert(!PreferencesContract); + +struct PreferencesMissingReset : public PreferencesMixin { + using PreferencesMixin::make_preference; + ESPPreferenceObject make_preference(size_t, uint32_t, bool) { return {}; } + ESPPreferenceObject make_preference(size_t, uint32_t) { return {}; } + bool sync() { return true; } +}; +static_assert(!PreferencesContract); + +struct PreferencesWrongSyncReturn : public PreferencesMixin { + using PreferencesMixin::make_preference; + ESPPreferenceObject make_preference(size_t, uint32_t, bool) { return {}; } + ESPPreferenceObject make_preference(size_t, uint32_t) { return {}; } + void sync() {} + bool reset() { return true; } +}; +static_assert(!PreferencesContract); + +// Forgot `using PreferencesMixin::make_preference;`, so the derived +// overloads hide the template forms (see the PreferencesContract note in +// preference_backend.h); the concept must reject the class. +struct PreferencesForgotUsingDeclaration : public PreferencesMixin { + ESPPreferenceObject make_preference(size_t, uint32_t, bool) { return {}; } + ESPPreferenceObject make_preference(size_t, uint32_t) { return {}; } + bool sync() { return true; } + bool reset() { return true; } +}; +static_assert(!PreferencesContract); + +struct MinimalKeyLookup { + bool load_from_key(uint32_t, uint8_t *, size_t) { return true; } +}; +static_assert(PreferencesKeyLookupContract); + +struct KeyLookupMissingMethod {}; +static_assert(!PreferencesKeyLookupContract); + +TEST(PreferenceContract, NullBackendRefusesBothOperations) { + // ESPPreferenceObject forwards to whichever backend the platform binds; a + // default-constructed object has no backend and must refuse both operations + // instead of crashing. + ESPPreferenceObject without_backend; + uint32_t value = 42; + EXPECT_FALSE(without_backend.save(&value)); + EXPECT_FALSE(without_backend.load(&value)); +} + +} // namespace esphome::core::testing diff --git a/tests/components/ethernet/common-ch390.yaml b/tests/components/ethernet/common-ch390.yaml new file mode 100644 index 0000000000..b27bc6ab4f --- /dev/null +++ b/tests/components/ethernet/common-ch390.yaml @@ -0,0 +1,19 @@ +ethernet: + type: CH390 + clk_pin: 19 + mosi_pin: 21 + miso_pin: 23 + cs_pin: 18 + interrupt_pin: 36 + reset_pin: 22 + clock_speed: 10Mhz + manual_ip: + static_ip: 192.168.178.56 + gateway: 192.168.178.1 + subnet: 255.255.255.0 + domain: .local + mac_address: "02:AA:BB:CC:DD:01" + on_connect: + - logger.log: "Ethernet connected!" + on_disconnect: + - logger.log: "Ethernet disconnected!" diff --git a/tests/components/ethernet/test-ch390.esp32-idf.yaml b/tests/components/ethernet/test-ch390.esp32-idf.yaml new file mode 100644 index 0000000000..50165d458f --- /dev/null +++ b/tests/components/ethernet/test-ch390.esp32-idf.yaml @@ -0,0 +1 @@ +<<: !include common-ch390.yaml diff --git a/tests/components/mitsubishi_cn105/climate/mitsubishi_cn105_tests.cpp b/tests/components/mitsubishi_cn105/climate/mitsubishi_cn105_tests.cpp index ef3cdd0fff..7703b02fcd 100644 --- a/tests/components/mitsubishi_cn105/climate/mitsubishi_cn105_tests.cpp +++ b/tests/components/mitsubishi_cn105/climate/mitsubishi_cn105_tests.cpp @@ -75,7 +75,7 @@ TEST(MitsubishiCN105Tests, ConnectAndUpdateStatus) { EXPECT_EQ(ctx.sut.status().vane_mode, MitsubishiCN105::VaneMode::POSITION_4); EXPECT_EQ(ctx.sut.status().wide_vane_mode, MitsubishiCN105::WideVaneMode::SWING); - // Now fetch room temperature (0x03) + // Now fetch telemetry (0x03) EXPECT_EQ(ctx.sut.state_, TestableMitsubishiCN105::State::UPDATING_STATUS); EXPECT_THAT(ctx.uart.tx, ::testing::ElementsAre(0xFC, 0x42, 0x01, 0x30, 0x10, 0x03, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x7A)); @@ -84,11 +84,11 @@ TEST(MitsubishiCN105Tests, ConnectAndUpdateStatus) { // Clear TX bytes. ctx.uart.tx.clear(); - // Room temperature response + // Telemetry response ctx.uart.push_rx({0xFC, 0x62, 0x01, 0x30, 0x10, 0x03, 0x00, 0x00, 0x0B, 0x00, 0x00, 0xAA, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0xA5}); - // Room temperature should still have initial value + // Room temperature from telemetry should still have initial value EXPECT_THAT(ctx.sut.status().room_temperature, ::testing::IsNan()); ctx.sut.set_current_time(400); @@ -97,7 +97,7 @@ TEST(MitsubishiCN105Tests, ConnectAndUpdateStatus) { EXPECT_TRUE(ctx.uart.rx.empty()); EXPECT_TRUE(ctx.sut.is_status_initialized()); - // Check room temperature we just read from received package + // Check room temperature we just read from telemetry package EXPECT_EQ(ctx.sut.status().room_temperature, 21.0f); EXPECT_TRUE(ctx.uart.tx.empty()); diff --git a/tests/components/mitsubishi_cn105/common.h b/tests/components/mitsubishi_cn105/common.h index 45f7b65289..a14043c737 100644 --- a/tests/components/mitsubishi_cn105/common.h +++ b/tests/components/mitsubishi_cn105/common.h @@ -8,6 +8,7 @@ #include #include "esphome/components/uart/uart_component.h" #include "esphome/components/mitsubishi_cn105/mitsubishi_cn105.h" +#include "esphome/components/mitsubishi_cn105/mitsubishi_cn105_component.h" #include "esphome/components/mitsubishi_cn105/mitsubishi_cn105_climate.h" namespace esphome::mitsubishi_cn105::testing { @@ -65,11 +66,16 @@ class TestableMitsubishiCN105 : public MitsubishiCN105 { class TestableMitsubishiCN105Climate : public MitsubishiCN105Climate { public: + TestableMitsubishiCN105Climate() { this->set_parent(&this->component_); } + using MitsubishiCN105Climate::apply_values_; using MitsubishiCN105Climate::last_non_swing_vane_mode_; using MitsubishiCN105Climate::last_non_swing_wide_vane_mode_; - MitsubishiCN105::Status &status() { return static_cast(this->hp_).status_; } + MitsubishiCN105::Status &status() { return const_cast(this->component_.status()); } + + protected: + MitsubishiCN105Component component_; }; } // namespace esphome::mitsubishi_cn105::testing diff --git a/tests/components/mitsubishi_cn105/common.yaml b/tests/components/mitsubishi_cn105/common.yaml index 5b9c3aaaf6..5966523b34 100644 --- a/tests/components/mitsubishi_cn105/common.yaml +++ b/tests/components/mitsubishi_cn105/common.yaml @@ -1,17 +1,20 @@ +mitsubishi_cn105: + id: ac + uart_id: uart_bus + update_interval: 30s + telemetry_request_min_interval: 120s + climate: - platform: mitsubishi_cn105 - id: ac + mitsubishi_cn105_id: ac name: "AC Test" - uart_id: uart_bus - update_interval: 30s - current_temperature_min_interval: 120s supported_swing_modes: BOTH esphome: on_boot: then: - - climate.mitsubishi_cn105.set_remote_temperature: + - mitsubishi_cn105.set_remote_temperature: id: ac temperature: 22.0 - - climate.mitsubishi_cn105.clear_remote_temperature: + - mitsubishi_cn105.clear_remote_temperature: id: ac diff --git a/tests/components/mitsubishi_cn105/test-legacy-climate-actions.esp32-idf.yaml b/tests/components/mitsubishi_cn105/test-legacy-climate-actions.esp32-idf.yaml new file mode 100644 index 0000000000..247568cfc3 --- /dev/null +++ b/tests/components/mitsubishi_cn105/test-legacy-climate-actions.esp32-idf.yaml @@ -0,0 +1,16 @@ +packages: + uart_9600_even: !include ../../test_build_components/common/uart_9600_even/esp32-idf.yaml + +climate: + - platform: mitsubishi_cn105 + id: ac + name: "AC Test" + +esphome: + on_boot: + then: + - climate.mitsubishi_cn105.set_remote_temperature: + id: ac + temperature: 22.0 + - climate.mitsubishi_cn105.clear_remote_temperature: + id: ac diff --git a/tests/components/mitsubishi_cn105/validate-legacy-climate-current-temperature-min-interval.esp32-idf.yaml b/tests/components/mitsubishi_cn105/validate-legacy-climate-current-temperature-min-interval.esp32-idf.yaml new file mode 100644 index 0000000000..a2abaf8b9b --- /dev/null +++ b/tests/components/mitsubishi_cn105/validate-legacy-climate-current-temperature-min-interval.esp32-idf.yaml @@ -0,0 +1,7 @@ +packages: + uart_9600_even: !include ../../test_build_components/common/uart_9600_even/esp32-idf.yaml + +climate: + - platform: mitsubishi_cn105 + name: "AC Test" + current_temperature_min_interval: 30s diff --git a/tests/components/mitsubishi_cn105/validate-legacy-climate-minimal.esp32-idf.yaml b/tests/components/mitsubishi_cn105/validate-legacy-climate-minimal.esp32-idf.yaml new file mode 100644 index 0000000000..0ef70b6535 --- /dev/null +++ b/tests/components/mitsubishi_cn105/validate-legacy-climate-minimal.esp32-idf.yaml @@ -0,0 +1,6 @@ +packages: + uart_9600_even: !include ../../test_build_components/common/uart_9600_even/esp32-idf.yaml + +climate: + - platform: mitsubishi_cn105 + name: "AC Test" diff --git a/tests/components/mitsubishi_cn105/validate-legacy-climate-uart-id.esp32-idf.yaml b/tests/components/mitsubishi_cn105/validate-legacy-climate-uart-id.esp32-idf.yaml new file mode 100644 index 0000000000..065d2b5495 --- /dev/null +++ b/tests/components/mitsubishi_cn105/validate-legacy-climate-uart-id.esp32-idf.yaml @@ -0,0 +1,7 @@ +packages: + uart_9600_even: !include ../../test_build_components/common/uart_9600_even/esp32-idf.yaml + +climate: + - platform: mitsubishi_cn105 + name: "AC Test" + uart_id: uart_bus diff --git a/tests/components/mitsubishi_cn105/validate-legacy-climate-update-interval.esp32-idf.yaml b/tests/components/mitsubishi_cn105/validate-legacy-climate-update-interval.esp32-idf.yaml new file mode 100644 index 0000000000..2e8f714f52 --- /dev/null +++ b/tests/components/mitsubishi_cn105/validate-legacy-climate-update-interval.esp32-idf.yaml @@ -0,0 +1,7 @@ +packages: + uart_9600_even: !include ../../test_build_components/common/uart_9600_even/esp32-idf.yaml + +climate: + - platform: mitsubishi_cn105 + name: "AC Test" + update_interval: 30s diff --git a/tests/components/mitsubishi_cn105/validate-top-level-minimal.esp32-idf.yaml b/tests/components/mitsubishi_cn105/validate-top-level-minimal.esp32-idf.yaml new file mode 100644 index 0000000000..03f05da5f4 --- /dev/null +++ b/tests/components/mitsubishi_cn105/validate-top-level-minimal.esp32-idf.yaml @@ -0,0 +1,8 @@ +packages: + uart_9600_even: !include ../../test_build_components/common/uart_9600_even/esp32-idf.yaml + +mitsubishi_cn105: + +climate: + - platform: mitsubishi_cn105 + name: "AC Test" diff --git a/tests/components/modbus/common.h b/tests/components/modbus/common.h index 659b72014c..d03ccf8ec3 100644 --- a/tests/components/modbus/common.h +++ b/tests/components/modbus/common.h @@ -1,5 +1,6 @@ #pragma once #include +#include #include "esphome/components/uart/uart_component.h" namespace esphome::modbus::testing { @@ -19,4 +20,14 @@ class NullUART : public uart::UARTComponent { void check_logger_conflict() override {} }; +// A UART that records every byte written so tests can assert on the exact wire response. +class RecordingUART : public NullUART { + public: + void write_array(const uint8_t *data, size_t len) override { + this->written.insert(this->written.end(), data, data + len); + } + + std::vector written; +}; + } // namespace esphome::modbus::testing diff --git a/tests/components/modbus/modbus_broadcast_test.cpp b/tests/components/modbus/modbus_broadcast_test.cpp index 5840259021..6f088f4888 100644 --- a/tests/components/modbus/modbus_broadcast_test.cpp +++ b/tests/components/modbus/modbus_broadcast_test.cpp @@ -28,6 +28,27 @@ class RecordingDevice : public ModbusServerDevice { std::vector last_values; }; +// A server device that records the coil writes the hub routes to it. Coils arrive as a PackedBits view +// over the hub's buffers, so the bits are copied out here rather than the view retained. +class RecordingCoilDevice : public ModbusServerDevice { + public: + explicit RecordingCoilDevice(uint8_t address) { this->set_address(address); } + + ResponseStatus on_write_coils(uint16_t start_address, PackedBits bits) override { + this->write_count++; + this->last_start_address = start_address; + this->last_bits.clear(); + for (uint16_t i = 0; i != bits.size(); i++) { + this->last_bits.push_back(bits[i]); + } + return std::nullopt; // return value is ignored for broadcasts, which are never answered + } + + int write_count{0}; + uint16_t last_start_address{0}; + std::vector last_bits; +}; + // A server device that rejects every write, to exercise the broadcast dispatch loop's rejection branch. class RejectingDevice : public ModbusServerDevice { public: @@ -41,15 +62,6 @@ class RejectingDevice : public ModbusServerDevice { int write_count{0}; }; -// A UART that records every byte written so the test can assert the hub sends no reply. -class RecordingUART : public testing::NullUART { - public: - void write_array(const uint8_t *data, size_t len) override { - this->written.insert(this->written.end(), data, data + len); - } - std::vector written; -}; - // Drives full frames through the server hub's receive path in tests. class TestServerHub : public ModbusServerHub { public: @@ -75,6 +87,8 @@ class TestServerHub : public ModbusServerHub { } // namespace +using testing::RecordingUART; + // A broadcast (address 0) single-register write reaches every registered device and is not answered. // Driven through the full receive parser (parse_modbus_frames) so the address-0 routing -- frame length, // CRC, and client-vs-broadcast dispatch -- is exercised, not just the handler below it. @@ -273,4 +287,83 @@ TEST(ModbusBroadcast, UnicastOutOfRangeWriteSendsSingleExceptionFrame) { EXPECT_EQ(uart.written[2], static_cast(ExceptionCode::ILLEGAL_DATA_ADDRESS)); } +// A broadcast single-coil write (FC 0x05) reaches every device and is not answered. The 2-byte ON value +// is normalized to a one-bit view, so the handler sees the same shape as a multiple-coil write of one. +TEST(ModbusBroadcast, SingleCoilWriteReachesAllDevicesWithoutReply) { + TestServerHub hub; + RecordingUART uart; + hub.set_uart_parent(&uart); + + RecordingCoilDevice device_a(0x02); + RecordingCoilDevice device_b(0x03); + hub.register_device(&device_a); + hub.register_device(&device_b); + + // FC 0x05 payload: coil 0x00AC, value 0xFF00 (ON). + const uint8_t pdu_data[] = {0x00, 0xAC, 0xFF, 0x00}; + ASSERT_TRUE(hub.run_receive_parser_for_test(BROADCAST_ADDRESS, static_cast(FunctionCode::WRITE_SINGLE_COIL), + pdu_data, sizeof(pdu_data))); + + for (RecordingCoilDevice *device : {&device_a, &device_b}) { + EXPECT_EQ(device->write_count, 1); + EXPECT_EQ(device->last_start_address, 0x00AC); + ASSERT_EQ(device->last_bits.size(), 1u); + EXPECT_TRUE(device->last_bits[0]); + } + EXPECT_TRUE(uart.written.empty()); // broadcasts are never answered +} + +// A broadcast multiple-coil write (FC 0x0F) delivers the packed bits to every device, LSB first. +TEST(ModbusBroadcast, MultipleCoilWriteReachesAllDevicesWithoutReply) { + TestServerHub hub; + RecordingUART uart; + hub.set_uart_parent(&uart); + + RecordingCoilDevice device_a(0x02); + RecordingCoilDevice device_b(0x03); + hub.register_device(&device_a); + hub.register_device(&device_b); + + // FC 0x0F payload: start 0x0013, 10 coils, 2 bytes, 0xCD 0x01 -> bit 0 set, bit 8 set. + const uint8_t pdu_data[] = {0x00, 0x13, 0x00, 0x0A, 0x02, 0xCD, 0x01}; + ASSERT_TRUE(hub.run_receive_parser_for_test( + BROADCAST_ADDRESS, static_cast(FunctionCode::WRITE_MULTIPLE_COILS), pdu_data, sizeof(pdu_data))); + + for (RecordingCoilDevice *device : {&device_a, &device_b}) { + EXPECT_EQ(device->write_count, 1); + EXPECT_EQ(device->last_start_address, 0x0013); + ASSERT_EQ(device->last_bits.size(), 10u); + EXPECT_TRUE(device->last_bits[0]); // 0xCD bit 0 + EXPECT_FALSE(device->last_bits[1]); // 0xCD bit 1 + EXPECT_TRUE(device->last_bits[8]); // 0x01 bit 0 + EXPECT_FALSE(device->last_bits[9]); // padding bit + } + EXPECT_TRUE(uart.written.empty()); +} + +// A coil broadcast that fails validation is dropped exactly like a bad register broadcast: no handler +// call and, because broadcasts are never answered, no exception frame either. +TEST(ModbusBroadcast, InvalidCoilBroadcastProducesNoWriteAndNoReply) { + TestServerHub hub; + RecordingUART uart; + hub.set_uart_parent(&uart); + + RecordingCoilDevice device(0x02); + hub.register_device(&device); + + // Byte count disagrees with the coil quantity: 10 coils need 2 bytes, not 1. + const uint8_t bad_count[] = {0x00, 0x13, 0x00, 0x0A, 0x01, 0xCD}; + ASSERT_TRUE(hub.run_receive_parser_for_test( + BROADCAST_ADDRESS, static_cast(FunctionCode::WRITE_MULTIPLE_COILS), bad_count, sizeof(bad_count))); + EXPECT_EQ(device.write_count, 0); + + // A single-coil value must be 0x0000 or 0xFF00; anything else is out of spec. + const uint8_t bad_value[] = {0x00, 0xAC, 0x12, 0x34}; + ASSERT_TRUE(hub.run_receive_parser_for_test(BROADCAST_ADDRESS, static_cast(FunctionCode::WRITE_SINGLE_COIL), + bad_value, sizeof(bad_value))); + EXPECT_EQ(device.write_count, 0); + + EXPECT_TRUE(uart.written.empty()); +} + } // namespace esphome::modbus diff --git a/tests/components/modbus/modbus_client_device_test.cpp b/tests/components/modbus/modbus_client_device_test.cpp index 38c28ce2df..333da5b228 100644 --- a/tests/components/modbus/modbus_client_device_test.cpp +++ b/tests/components/modbus/modbus_client_device_test.cpp @@ -118,6 +118,35 @@ TEST(ModbusClientDeviceFanOut, ReadHoldingRegistersSuccess) { EXPECT_FALSE(call.status.has_value()); } +// FC 0x17: the response carries only the read block, so it decodes as a holding-register read of the read +// start/count. The write half has no client-side ack callback - it is confirmed by a successful response. +TEST(ModbusClientDeviceFanOut, ReadWriteMultipleRegistersDeliversReadBlockAsHolding) { + RecordingDevice device; + // read 2 regs at 0x0010, write 1 reg (0x00FF) at 0x0020 + const uint8_t request[] = {0x17, 0x00, 0x10, 0x00, 0x02, 0x00, 0x20, 0x00, 0x01, 0x02, 0x00, 0xFF}; + const uint8_t response[] = {0x17, 0x04, 0x00, 0x2A, 0x01, 0x00}; // read-back: 0x002A, 0x0100 + device.on_response(request, response); + + ASSERT_EQ(device.holding_calls.size(), 1u); + const auto &call = device.holding_calls.front(); + EXPECT_EQ(call.start_address, 0x0010); // the READ start address, not the write + EXPECT_EQ(call.registers, (std::vector{0x002A, 0x0100})); + EXPECT_FALSE(call.status.has_value()); + EXPECT_TRUE(device.write_multiple_registers_calls.empty()); // no separate write-ack on the client side +} + +// A 0x17 response shorter than the requested read count is self-consistent but wrong; it must be diverted +// to on_custom_response(), never clamped and delivered as if complete. +TEST(ModbusClientDeviceFanOut, ReadWriteMultipleRegistersShortResponseGoesToCustom) { + RecordingDevice device; + const uint8_t request[] = {0x17, 0x00, 0x10, 0x00, 0x02, 0x00, 0x20, 0x00, 0x01, 0x02, 0x00, 0xFF}; + const uint8_t response[] = {0x17, 0x02, 0x00, 0x2A}; // only 1 register, but 2 were requested + device.on_response(request, response); + + EXPECT_TRUE(device.holding_calls.empty()); + EXPECT_EQ(device.custom_requests.size(), 1u); +} + TEST(ModbusClientDeviceFanOut, ReadInputRegistersDelegateToGeneric) { GenericDevice device; const uint8_t request[] = {0x04, 0x00, 0x10, 0x00, 0x01}; diff --git a/tests/components/modbus/modbus_helpers_test.cpp b/tests/components/modbus/modbus_helpers_test.cpp index 768c23c33c..53f51b016b 100644 --- a/tests/components/modbus/modbus_helpers_test.cpp +++ b/tests/components/modbus/modbus_helpers_test.cpp @@ -426,6 +426,20 @@ TEST(ModbusHelpersTest, RegistersToNumberRejectsTruncatedMultiRegisterValue) { EXPECT_FALSE(registers_to_number(registers, 1, SensorValueType::U_DWORD).has_value()); } +// --- packed bit helpers ------------------------------------------------------ + +TEST(ModbusHelpersTest, PackBitsAppendsToContainer) { + // Bits are packed LSB first: the first value is bit 0 of the first byte, and the push_back + // overload appends packed bytes onto a growable container preserving existing content. + std::vector bits{true, false, true, true, false, false, false, false, true, true}; + std::vector out{0x55}; // pre-existing content must be preserved + pack_bits(out, bits); + ASSERT_EQ(out.size(), 3u); // leading byte + 2 packed bytes (10 bits) + EXPECT_EQ(out[0], 0x55); + EXPECT_EQ(out[1], 0x0D); // 0b00001101 + EXPECT_EQ(out[2], 0x03); // bits 8 and 9 -> bits 0,1 of second byte +} + // --- typed builders ---------------------------------------------------------- TEST(ModbusTypedBuilders, ReadPduWireBytes) { @@ -469,6 +483,71 @@ TEST(ModbusTypedBuilders, WriteRegistersPduRejectsOverLimit) { EXPECT_FALSE(create_write_registers_pdu(0x0000, values).empty()); } +TEST(ModbusTypedBuilders, ReadWriteMultipleRegistersPduWireBytes) { + const uint16_t write_values[] = {0x000B, 0x0016}; + // Read 2 registers at 0x0010, write 2 registers at 0x0020. + auto pdu = create_read_write_multiple_registers_pdu(0x0010, 2, 0x0020, write_values); + const std::vector expected{0x17, 0x00, 0x10, 0x00, 0x02, 0x00, 0x20, + 0x00, 0x02, 0x04, 0x00, 0x0B, 0x00, 0x16}; + EXPECT_EQ(std::vector(pdu.begin(), pdu.end()), expected); + EXPECT_TRUE(is_client_pdu_standard(pdu.data(), pdu.size())); +} + +TEST(ModbusTypedBuilders, ReadWriteMultipleRegistersPduRejectsOutOfRange) { + const uint16_t one_value[] = {0x0001}; + const uint16_t two_values[] = {0x0001, 0x0002}; + // Read count out of range (zero and above the read ceiling). + EXPECT_TRUE(create_read_write_multiple_registers_pdu(0x0000, 0, 0x0020, one_value).empty()); + EXPECT_TRUE( + create_read_write_multiple_registers_pdu(0x0000, MAX_NUM_OF_REGISTERS_TO_READ + 1, 0x0020, one_value).empty()); + // Write count out of range (empty, and above the read/write ceiling which is lower than a plain write). + EXPECT_TRUE(create_read_write_multiple_registers_pdu(0x0000, 1, 0x0020, std::span()).empty()); + std::vector too_many(MAX_NUM_OF_REGISTERS_TO_WRITE_RW + 1, 0xAAAA); + EXPECT_TRUE(create_read_write_multiple_registers_pdu(0x0000, 1, 0x0020, too_many).empty()); + // Both blocks at their respective ceilings are accepted. + std::vector at_write_limit(MAX_NUM_OF_REGISTERS_TO_WRITE_RW, 0xAAAA); + EXPECT_FALSE( + create_read_write_multiple_registers_pdu(0x0000, MAX_NUM_OF_REGISTERS_TO_READ, 0x0020, at_write_limit).empty()); + // A block that runs past the 16-bit address space is refused (read block, then write block). + EXPECT_TRUE(create_read_write_multiple_registers_pdu(0xFFFF, 2, 0x0020, one_value).empty()); + EXPECT_TRUE(create_read_write_multiple_registers_pdu(0x0000, 2, 0xFFFF, two_values).empty()); + // Accept boundary: a block ending exactly at 0x10000 (last register 0xFFFF) still fits. + EXPECT_FALSE(create_read_write_multiple_registers_pdu(0xFFFE, 2, 0x0000, one_value).empty()); // read ends at 0x10000 + EXPECT_FALSE( + create_read_write_multiple_registers_pdu(0x0000, 1, 0xFFFF, one_value).empty()); // write ends at 0x10000 +} + +TEST(ModbusFunctionCodeClass, ReadWriteMultipleCountsAsBothReadAndWrite) { + const auto rw = static_cast(FC::READ_WRITE_MULTIPLE_REGISTERS); + // 0x17 both reads and writes, but it is not a pure (retry-safe) read. + EXPECT_TRUE(is_function_code_read(rw)); + EXPECT_TRUE(is_function_code_write(rw)); + EXPECT_FALSE(is_function_code_read_only(rw)); + // Pure reads are read and read-only, never write. + const auto rd = static_cast(FC::READ_HOLDING_REGISTERS); + EXPECT_TRUE(is_function_code_read(rd)); + EXPECT_TRUE(is_function_code_read_only(rd)); + EXPECT_FALSE(is_function_code_write(rd)); + // Plain writes are write only. + const auto wr = static_cast(FC::WRITE_MULTIPLE_REGISTERS); + EXPECT_TRUE(is_function_code_write(wr)); + EXPECT_FALSE(is_function_code_read(wr)); + EXPECT_FALSE(is_function_code_read_only(wr)); + // Mask-write register mutates via read-modify-write, so it classes as a write, never a read. + const auto mask = static_cast(FC::MASK_WRITE_REGISTER); + EXPECT_TRUE(is_function_code_write(mask)); + EXPECT_FALSE(is_function_code_read(mask)); + EXPECT_FALSE(is_function_code_read_only(mask)); +} + +TEST(ModbusCreateClientPdu, ReadWriteMultipleReturnsEmpty) { + // The generic builder cannot express 0x17's two blocks; callers use the dedicated builder instead. + const uint16_t values[] = {0x0001}; + EXPECT_TRUE(create_client_pdu(FC::READ_WRITE_MULTIPLE_REGISTERS, 0x0000, 1, reinterpret_cast(values), + sizeof(values)) + .empty()); +} + TEST(ModbusTypedBuilders, FloatToPayloadAppendsToExistingContent) { // The container overload appends - the semantic every migrated caller relies on when a lambda // has already put words into the buffer. diff --git a/tests/components/modbus/modbus_server_coils_test.cpp b/tests/components/modbus/modbus_server_coils_test.cpp new file mode 100644 index 0000000000..e4bf3f6b14 --- /dev/null +++ b/tests/components/modbus/modbus_server_coils_test.cpp @@ -0,0 +1,398 @@ +#include + +#include +#include +#include + +#include "common.h" +#include "esphome/components/modbus/modbus.h" +#include "esphome/core/hal.h" + +namespace esphome::modbus { + +namespace { + +// A server device backed by a small coil array: reads deliver the stored bits, writes apply them. +class CoilDevice : public ModbusServerDevice { + public: + explicit CoilDevice(uint8_t address) { this->set_address(address); } + + ResponseStatus on_read_coils(uint16_t start_address, MutablePackedBits bits) override { + this->read_count++; + for (uint16_t i = 0; i < bits.size(); i++) + bits.set(i, this->coils[start_address + i]); + return std::nullopt; + } + + ResponseStatus on_write_coils(uint16_t start_address, PackedBits bits) override { + this->write_count++; + this->last_write_count = bits.size(); + for (uint16_t i = 0; i < bits.size(); i++) + this->coils[start_address + i] = bits[i]; + return std::nullopt; + } + + bool coils[32] = {}; + int read_count{0}; + int write_count{0}; + uint16_t last_write_count{0}; +}; + +// A device with no bit handlers, to exercise the ILLEGAL_FUNCTION defaults. +class NoBitsDevice : public ModbusServerDevice { + public: + explicit NoBitsDevice(uint8_t address) { this->set_address(address); } +}; + +// Distinguishes the two bit-read entry points: each fills a different pattern and counts its calls, so a +// test can prove FC 0x01 vs 0x02 dispatch routes to the right handler (and not merely that bits came back). +class DualReadDevice : public ModbusServerDevice { + public: + explicit DualReadDevice(uint8_t address) { this->set_address(address); } + + ResponseStatus on_read_coils(uint16_t start_address, MutablePackedBits bits) override { + this->coil_reads++; + bits.set(0, true); // pattern 0x01 + return std::nullopt; + } + ResponseStatus on_read_discrete_inputs(uint16_t start_address, MutablePackedBits bits) override { + this->discrete_reads++; + bits.set(1, true); // pattern 0x02 + return std::nullopt; + } + + int coil_reads{0}; + int discrete_reads{0}; +}; + +// Overrides only on_read_bits() - the shared fallback the header documents that on_read_coils() and +// on_read_discrete_inputs() default to. Both FC 0x01 and FC 0x02 must reach it. +class BitsOnlyDevice : public ModbusServerDevice { + public: + explicit BitsOnlyDevice(uint8_t address) { this->set_address(address); } + ResponseStatus on_read_bits(uint16_t start_address, MutablePackedBits bits) override { + this->calls++; + bits.set(0, true); // set bit 0 so the response proves the fallback ran + return std::nullopt; + } + int calls{0}; +}; + +using testing::RecordingUART; + +// Exposes the client-frame parser so a fully CRC-framed request can be pushed through the hub. +class TestServerHub : public ModbusServerHub { + public: + bool tx_blocked() override { return false; } + + void prime_send_timestamps_for_test() { + uint32_t now = millis(); + this->last_modbus_byte_ = now; + this->last_send_ = now; + } + + bool process_full_client_frame_for_test(uint8_t address, uint8_t function_code, const uint8_t *pdu_data, + size_t pdu_data_len) { + this->rx_buffer_.clear(); + this->rx_buffer_.reserve(pdu_data_len + 4); + this->rx_buffer_.push_back(address); + this->rx_buffer_.push_back(function_code); + this->rx_buffer_.insert(this->rx_buffer_.end(), pdu_data, pdu_data + pdu_data_len); + uint16_t crc = crc16(this->rx_buffer_.data(), this->rx_buffer_.size()); + this->rx_buffer_.push_back(crc & 0xFF); + this->rx_buffer_.push_back(crc >> 8); + return this->parse_modbus_client_frame_(); + } +}; + +struct CoilFixture { + CoilFixture() { + hub.set_uart_parent(&uart); + hub.prime_send_timestamps_for_test(); + hub.register_device(&device); + } + TestServerHub hub; + RecordingUART uart; + CoilDevice device{0x02}; +}; + +} // namespace + +// A coil read returns byte count + packed bits, set by the handler directly in the response buffer. +TEST(ModbusServerCoils, ReadCoilsReturnsPackedBits) { + CoilFixture f; + f.device.coils[0] = true; + f.device.coils[2] = true; + f.device.coils[3] = true; + f.device.coils[9] = true; + + // FC 0x01: start 0x0000, quantity 10 -> 2 packed bytes + const uint8_t pdu_data[] = {0x00, 0x00, 0x00, 0x0A}; + ASSERT_TRUE(f.hub.process_full_client_frame_for_test(0x02, static_cast(FunctionCode::READ_COILS), pdu_data, + sizeof(pdu_data))); + + EXPECT_EQ(f.device.read_count, 1); + // Response: address(1) + fc(1) + byte count(1) + packed(2) + CRC(2) + ASSERT_EQ(f.uart.written.size(), 7u); + EXPECT_EQ(f.uart.written[0], 0x02); + EXPECT_EQ(f.uart.written[1], static_cast(FunctionCode::READ_COILS)); + EXPECT_EQ(f.uart.written[2], 2u); // byte count + EXPECT_EQ(f.uart.written[3], 0x0D); // coils 0,2,3 + EXPECT_EQ(f.uart.written[4], 0x02); // coil 9 -> bit 1 of byte 1 +} + +// A device overriding only on_read_bits() - the documented fallback - still serves both FC 0x01 (coils) +// and FC 0x02 (discrete inputs), since on_read_coils()/on_read_discrete_inputs() default to it. +TEST(ModbusServerCoils, ReadBitsFallbackServesBothCoilsAndDiscreteInputs) { + TestServerHub hub; + RecordingUART uart; + hub.set_uart_parent(&uart); + hub.prime_send_timestamps_for_test(); + BitsOnlyDevice device{0x05}; + hub.register_device(&device); + + const uint8_t pdu_data[] = {0x00, 0x00, 0x00, 0x01}; // start 0x0000, quantity 1 + + ASSERT_TRUE(hub.process_full_client_frame_for_test(0x05, static_cast(FunctionCode::READ_COILS), pdu_data, + sizeof(pdu_data))); + EXPECT_EQ(device.calls, 1); + // address(1) + fc(1) + byte count(1) + packed(1) + CRC(2); bit 0 set -> 0x01 + ASSERT_EQ(uart.written.size(), 6u); + EXPECT_EQ(uart.written[1], static_cast(FunctionCode::READ_COILS)); + EXPECT_EQ(uart.written[3], 0x01); + + uart.written.clear(); + ASSERT_TRUE(hub.process_full_client_frame_for_test(0x05, static_cast(FunctionCode::READ_DISCRETE_INPUTS), + pdu_data, sizeof(pdu_data))); + EXPECT_EQ(device.calls, 2); + ASSERT_EQ(uart.written.size(), 6u); + EXPECT_EQ(uart.written[1], static_cast(FunctionCode::READ_DISCRETE_INPUTS)); + EXPECT_EQ(uart.written[3], 0x01); +} + +// A multiple-coil write hands the handler the packed wire bytes and echoes the request header. +TEST(ModbusServerCoils, WriteMultipleCoilsAppliesPackedBits) { + CoilFixture f; + + // FC 0x0F: start 0x0000, quantity 10, byte count 2, packed values 0x0D 0x02 + const uint8_t pdu_data[] = {0x00, 0x00, 0x00, 0x0A, 0x02, 0x0D, 0x02}; + ASSERT_TRUE(f.hub.process_full_client_frame_for_test(0x02, static_cast(FunctionCode::WRITE_MULTIPLE_COILS), + pdu_data, sizeof(pdu_data))); + + EXPECT_EQ(f.device.write_count, 1); + EXPECT_EQ(f.device.last_write_count, 10u); + EXPECT_TRUE(f.device.coils[0]); + EXPECT_FALSE(f.device.coils[1]); + EXPECT_TRUE(f.device.coils[2]); + EXPECT_TRUE(f.device.coils[3]); + EXPECT_TRUE(f.device.coils[9]); + EXPECT_FALSE(f.device.coils[10]); + // Response echoes start address + quantity: address(1) + fc(1) + start(2) + quantity(2) + CRC(2) + ASSERT_EQ(f.uart.written.size(), 8u); + EXPECT_EQ(f.uart.written[1], static_cast(FunctionCode::WRITE_MULTIPLE_COILS)); +} + +// A single-coil write (FC 0x05) is normalized to a one-bit packed buffer. +TEST(ModbusServerCoils, WriteSingleCoilNormalizedToOneBit) { + CoilFixture f; + + const uint8_t pdu_on[] = {0x00, 0x03, 0xFF, 0x00}; // coil 3 ON + ASSERT_TRUE(f.hub.process_full_client_frame_for_test(0x02, static_cast(FunctionCode::WRITE_SINGLE_COIL), + pdu_on, sizeof(pdu_on))); + EXPECT_EQ(f.device.last_write_count, 1u); + EXPECT_TRUE(f.device.coils[3]); + + f.uart.written.clear(); + f.hub.prime_send_timestamps_for_test(); + const uint8_t pdu_off[] = {0x00, 0x03, 0x00, 0x00}; // coil 3 OFF + ASSERT_TRUE(f.hub.process_full_client_frame_for_test(0x02, static_cast(FunctionCode::WRITE_SINGLE_COIL), + pdu_off, sizeof(pdu_off))); + EXPECT_FALSE(f.device.coils[3]); + EXPECT_EQ(f.device.write_count, 2); +} + +// An invalid single-coil value (not 0xFF00/0x0000) is rejected with ILLEGAL_DATA_VALUE, no write. +TEST(ModbusServerCoils, InvalidSingleCoilValueRejected) { + CoilFixture f; + + const uint8_t pdu_data[] = {0x00, 0x03, 0x12, 0x34}; + ASSERT_TRUE(f.hub.process_full_client_frame_for_test(0x02, static_cast(FunctionCode::WRITE_SINGLE_COIL), + pdu_data, sizeof(pdu_data))); + + EXPECT_EQ(f.device.write_count, 0); + ASSERT_EQ(f.uart.written.size(), 5u); // one exception frame + EXPECT_EQ(f.uart.written[1], static_cast(FunctionCode::WRITE_SINGLE_COIL) | 0x80); + EXPECT_EQ(f.uart.written[2], static_cast(ExceptionCode::ILLEGAL_DATA_VALUE)); +} + +// Read quantity validation lives in the shared read-request parser, so the register and bit reads cannot +// drift apart. These pin both ends of the range for coils; the register case below pins that the same +// parser is on that path too. +TEST(ModbusServerCoils, ZeroCoilReadQuantityRejected) { + CoilFixture f; + + // FC 0x01: start 0x0000, quantity 0 - a read of nothing is out of spec. + const uint8_t pdu_data[] = {0x00, 0x00, 0x00, 0x00}; + ASSERT_TRUE(f.hub.process_full_client_frame_for_test(0x02, static_cast(FunctionCode::READ_COILS), pdu_data, + sizeof(pdu_data))); + + EXPECT_EQ(f.device.read_count, 0); + ASSERT_EQ(f.uart.written.size(), 5u); // one exception frame + EXPECT_EQ(f.uart.written[1], static_cast(FunctionCode::READ_COILS) | 0x80); + EXPECT_EQ(f.uart.written[2], static_cast(ExceptionCode::ILLEGAL_DATA_VALUE)); +} + +TEST(ModbusServerCoils, OverLimitCoilReadQuantityRejected) { + CoilFixture f; + + // One past MAX_NUM_OF_COILS_TO_READ (2000 = 0x07D0), which no frame could carry anyway. + const uint8_t pdu_data[] = {0x00, 0x00, 0x07, 0xD1}; + ASSERT_TRUE(f.hub.process_full_client_frame_for_test(0x02, static_cast(FunctionCode::READ_COILS), pdu_data, + sizeof(pdu_data))); + + EXPECT_EQ(f.device.read_count, 0); + ASSERT_EQ(f.uart.written.size(), 5u); + EXPECT_EQ(f.uart.written[2], static_cast(ExceptionCode::ILLEGAL_DATA_VALUE)); +} + +// The register read path shares that parser, so a zero quantity is rejected there identically. Lives +// beside the coil cases deliberately: together they are what stops the shared parser being bypassed on +// one side without the other noticing. +TEST(ModbusServerCoils, ZeroRegisterReadQuantityRejectedByTheSameParser) { + CoilFixture f; + + const uint8_t pdu_data[] = {0x00, 0x00, 0x00, 0x00}; + ASSERT_TRUE(f.hub.process_full_client_frame_for_test(0x02, static_cast(FunctionCode::READ_HOLDING_REGISTERS), + pdu_data, sizeof(pdu_data))); + + ASSERT_EQ(f.uart.written.size(), 5u); + EXPECT_EQ(f.uart.written[1], static_cast(FunctionCode::READ_HOLDING_REGISTERS) | 0x80); + EXPECT_EQ(f.uart.written[2], static_cast(ExceptionCode::ILLEGAL_DATA_VALUE)); +} + +// A device without bit handlers rejects coil requests with ILLEGAL_FUNCTION via the defaults. +TEST(ModbusServerCoils, UnhandledCoilReadIsIllegalFunction) { + TestServerHub hub; + RecordingUART uart; + hub.set_uart_parent(&uart); + hub.prime_send_timestamps_for_test(); + NoBitsDevice device(0x02); + hub.register_device(&device); + + const uint8_t pdu_data[] = {0x00, 0x00, 0x00, 0x08}; + ASSERT_TRUE(hub.process_full_client_frame_for_test(0x02, static_cast(FunctionCode::READ_COILS), pdu_data, + sizeof(pdu_data))); + + ASSERT_EQ(uart.written.size(), 5u); + EXPECT_EQ(uart.written[1], static_cast(FunctionCode::READ_COILS) | 0x80); + EXPECT_EQ(uart.written[2], static_cast(ExceptionCode::ILLEGAL_FUNCTION)); +} + +// The view contracts are enforced, not merely documented: bytes() returns exactly ceil(size()/8) bytes +// even over a larger buffer (forwarding it can never leak trailing buffer content), and set() drops +// out-of-range bits instead of writing past the span (on the server read path that span wraps a stack +// response buffer). +TEST(ModbusServerCoils, PackedBitsViewContractsEnforced) { + uint8_t buf[8] = {}; + PackedBits view(buf, 10); // 10 bits -> 2 bytes, over an 8-byte buffer + EXPECT_EQ(view.bytes().size(), 2u); + + PackedBits short_view(std::span(buf, 1), 10); // contract-violating: 10 bits over 1 byte + EXPECT_EQ(short_view.bytes().size(), 1u); // clamped to the real span, not a fabricated 2-byte span + + MutablePackedBits bits(std::span(buf, 2), 10); + bits.set(9, true); // in range: lands in byte 1 + bits.set(10, true); // out of range: dropped + bits.set(300, true); // far out of range: dropped, no write past the span + EXPECT_EQ(buf[1], 0x02); + for (size_t i = 2; i < sizeof(buf); i++) + EXPECT_EQ(buf[i], 0) << "byte " << i; +} + +// FC 0x02 must dispatch to on_read_discrete_inputs, not on_read_coils: the two handlers fill different +// patterns, so a swapped dispatch would fail on both the counters and the wire bytes. +TEST(ModbusServerCoils, ReadDiscreteInputsDispatchesToItsOwnHandler) { + TestServerHub hub; + RecordingUART uart; + hub.set_uart_parent(&uart); + hub.prime_send_timestamps_for_test(); + DualReadDevice device(0x02); + hub.register_device(&device); + + const uint8_t pdu_data[] = {0x00, 0x00, 0x00, 0x08}; + ASSERT_TRUE(hub.process_full_client_frame_for_test(0x02, static_cast(FunctionCode::READ_DISCRETE_INPUTS), + pdu_data, sizeof(pdu_data))); + + EXPECT_EQ(device.discrete_reads, 1); + EXPECT_EQ(device.coil_reads, 0); + ASSERT_GE(uart.written.size(), 4u); + EXPECT_EQ(uart.written[1], static_cast(FunctionCode::READ_DISCRETE_INPUTS)); + EXPECT_EQ(uart.written[3], 0x02); // the discrete handler's pattern, not the coil handler's + + uart.written.clear(); + ASSERT_TRUE(hub.process_full_client_frame_for_test(0x02, static_cast(FunctionCode::READ_COILS), pdu_data, + sizeof(pdu_data))); + EXPECT_EQ(device.coil_reads, 1); + EXPECT_EQ(device.discrete_reads, 1); + ASSERT_GE(uart.written.size(), 4u); + EXPECT_EQ(uart.written[3], 0x01); +} + +// The write-side ILLEGAL_FUNCTION defaults: a device without bit handlers rejects coil writes too +// (single and multiple), mirroring the read-side default already covered above. +TEST(ModbusServerCoils, UnhandledCoilWriteIsIllegalFunction) { + TestServerHub hub; + RecordingUART uart; + hub.set_uart_parent(&uart); + hub.prime_send_timestamps_for_test(); + NoBitsDevice device(0x02); + hub.register_device(&device); + + const uint8_t single[] = {0x00, 0x03, 0xFF, 0x00}; + ASSERT_TRUE(hub.process_full_client_frame_for_test(0x02, static_cast(FunctionCode::WRITE_SINGLE_COIL), + single, sizeof(single))); + ASSERT_EQ(uart.written.size(), 5u); + EXPECT_EQ(uart.written[1], static_cast(FunctionCode::WRITE_SINGLE_COIL) | 0x80); + EXPECT_EQ(uart.written[2], static_cast(ExceptionCode::ILLEGAL_FUNCTION)); + + uart.written.clear(); + const uint8_t multiple[] = {0x00, 0x00, 0x00, 0x08, 0x01, 0xAA}; + ASSERT_TRUE(hub.process_full_client_frame_for_test(0x02, static_cast(FunctionCode::WRITE_MULTIPLE_COILS), + multiple, sizeof(multiple))); + ASSERT_EQ(uart.written.size(), 5u); + EXPECT_EQ(uart.written[1], static_cast(FunctionCode::WRITE_MULTIPLE_COILS) | 0x80); + EXPECT_EQ(uart.written[2], static_cast(ExceptionCode::ILLEGAL_FUNCTION)); +} + +// FC 0x0F with a byte count that does not match ceil(quantity / 8) is ILLEGAL_DATA_VALUE and never +// reaches the handler. +TEST(ModbusServerCoils, WriteCoilsByteCountMismatchRejected) { + CoilFixture f; + + // quantity 10 needs 2 bytes; claim 1 + const uint8_t pdu_data[] = {0x00, 0x00, 0x00, 0x0A, 0x01, 0xFF}; + ASSERT_TRUE(f.hub.process_full_client_frame_for_test(0x02, static_cast(FunctionCode::WRITE_MULTIPLE_COILS), + pdu_data, sizeof(pdu_data))); + + ASSERT_EQ(f.uart.written.size(), 5u); + EXPECT_EQ(f.uart.written[1], static_cast(FunctionCode::WRITE_MULTIPLE_COILS) | 0x80); + EXPECT_EQ(f.uart.written[2], static_cast(ExceptionCode::ILLEGAL_DATA_VALUE)); + EXPECT_EQ(f.device.write_count, 0); +} + +// A coil range that runs past address 0xFFFF is ILLEGAL_DATA_ADDRESS and never reaches the handler. +TEST(ModbusServerCoils, CoilAddressRangeOverflowRejected) { + CoilFixture f; + + // start 0xFFF8, quantity 16 -> 0x10008 > 0x10000 + const uint8_t pdu_data[] = {0xFF, 0xF8, 0x00, 0x10}; + ASSERT_TRUE(f.hub.process_full_client_frame_for_test(0x02, static_cast(FunctionCode::READ_COILS), pdu_data, + sizeof(pdu_data))); + + ASSERT_EQ(f.uart.written.size(), 5u); + EXPECT_EQ(f.uart.written[1], static_cast(FunctionCode::READ_COILS) | 0x80); + EXPECT_EQ(f.uart.written[2], static_cast(ExceptionCode::ILLEGAL_DATA_ADDRESS)); + EXPECT_EQ(f.device.read_count, 0); +} + +} // namespace esphome::modbus diff --git a/tests/components/modbus_client/common.yaml b/tests/components/modbus_client/common.yaml index cae2002342..bce2149fbf 100644 --- a/tests/components/modbus_client/common.yaml +++ b/tests/components/modbus_client/common.yaml @@ -35,6 +35,8 @@ button: id(bare_client).write_single_register(0x10, 42); id(bare_client).write_single_coil(0x01, true); id(bare_client_explicit_hub).read_holding_registers(0x20, 4); + const uint16_t rw_vals[] = {1, 2}; + id(bare_client).read_write_multiple_registers(0x0400, 2, 0x0300, rw_vals); - platform: template name: "Send Read" on_press: @@ -134,3 +136,13 @@ button: on_error: then: - lambda: 'ESP_LOGW("modbus_client.test", "fc 0x%X exception %d", request.empty() ? 0 : request[0], (int) exception_code);' + - modbus_client.read_write_multiple_registers: + address: 0x01 + write_address: 0x0300 + values: !lambda "return {1, 2};" + read_address: 0x0400 + read_count: 2 + on_response: + then: + # `values` here is the READ-BACK block, not the written block above + - lambda: 'ESP_LOGI("modbus_client.test", "rw read0=%u n=%u", values[0], (unsigned) values.size());' diff --git a/tests/components/modbus_controller/command_payload_test.cpp b/tests/components/modbus_controller/command_payload_test.cpp new file mode 100644 index 0000000000..c125a44da5 --- /dev/null +++ b/tests/components/modbus_controller/command_payload_test.cpp @@ -0,0 +1,31 @@ +#include + +#include +#include + +#include "esphome/components/modbus_controller/modbus_controller.h" + +namespace esphome::modbus_controller::testing { + +// The coil write factory packs into an exact-size payload. Pinned at one past the protocol maximum +// because a fixed pack buffer sized for the maximum would silently truncate there while the quantity +// field still claimed every coil - and the truncated frame would fit the RTU limit and go on the wire +// malformed. Built at its true byte count, the oversize frame is refused by the hub's size check with +// a log instead. +TEST(ModbusCommandPayload, CoilWritePayloadIsExactSizedNotTruncated) { + ModbusController controller; + std::vector coils(modbus::MAX_NUM_OF_COILS_TO_WRITE + 1, true); + auto cmd = ModbusCommandItem::create_write_multiple_coils(&controller, 0x10, coils); + EXPECT_EQ(cmd.payload.size(), modbus::packed_bit_bytes(coils.size())); +} + +// LSB-first packing with zeroed pad bits, matching the wire layout the PDU builders produce. +TEST(ModbusCommandPayload, CoilWritePacksLsbFirstWithZeroPad) { + ModbusController controller; + const std::vector coils{true, false, true, true}; + auto cmd = ModbusCommandItem::create_write_multiple_coils(&controller, 0x10, coils); + ASSERT_EQ(cmd.payload.size(), 1u); + EXPECT_EQ(cmd.payload.data()[0], 0b00001101); +} + +} // namespace esphome::modbus_controller::testing diff --git a/tests/components/ota/test_backend_contract.cpp b/tests/components/ota/test_backend_contract.cpp new file mode 100644 index 0000000000..1b4fbbc32d --- /dev/null +++ b/tests/components/ota/test_backend_contract.cpp @@ -0,0 +1,49 @@ +// Pins the OTA backend contract concept so the surface it enforces cannot +// drift unnoticed: the build's real backend and a minimal conforming type +// must satisfy it, and a type missing a method or returning the wrong type +// must not. + +#include "esphome/components/ota/ota_backend.h" +#include "esphome/components/ota/ota_backend_host.h" + +namespace esphome::ota::testing { + +struct MinimalBackend { + OTAResponseTypes begin(size_t image_size, OTAType ota_type = OTA_TYPE_UPDATE_APP) { return OTA_RESPONSE_OK; } + void set_update_md5(const char *md5) {} + OTAResponseTypes write(uint8_t *data, size_t len) { return OTA_RESPONSE_OK; } + OTAResponseTypes end() { return OTA_RESPONSE_OK; } + void abort() {} + bool supports_compression() { return false; } +}; +static_assert(OTABackendContract); + +// Each negative case derives from MinimalBackend and breaks exactly one +// requirement; the declaration in the derived struct hides the conforming +// one from the base. + +// begin() without the default ota_type argument breaks consumers that only +// pass the image size. +struct BackendWithoutDefaultOTAType : MinimalBackend { + OTAResponseTypes begin(size_t image_size, OTAType ota_type) { return OTA_RESPONSE_OK; } +}; +static_assert(!OTABackendContract); + +struct BackendMissingAbort : MinimalBackend { + void abort() = delete; +}; +static_assert(!OTABackendContract); + +struct BackendWrongWriteReturn : MinimalBackend { + bool write(uint8_t *data, size_t len) { return true; } +}; +static_assert(!OTABackendContract); + +// Pin the build's real backend, not just local mocks: the unit test harness +// builds for the host platform, so this is the same check the factory's +// static_assert performs in a firmware compile. +#ifdef USE_HOST +static_assert(OTABackendContract); +#endif + +} // namespace esphome::ota::testing diff --git a/tests/components/ufm01/common.h b/tests/components/ufm01/common.h new file mode 100644 index 0000000000..1582358700 --- /dev/null +++ b/tests/components/ufm01/common.h @@ -0,0 +1,156 @@ +#pragma once + +#include +#include +#include +#include + +#include + +#include "esphome/components/uart/uart_component.h" +#include "esphome/components/ufm01/ufm01.h" + +namespace esphome::ufm01::testing { + +static constexpr uint8_t FRAME_START_BYTE_1 = 0x3C; +static constexpr uint8_t FRAME_START_BYTE_2 = 0x32; +static constexpr uint8_t PASSIVE_START_BYTE_2 = 0x64; +static constexpr uint8_t FRAME_STOP_BYTE = 0x16; +static constexpr uint8_t FRAME_FLAG_INSTANT_FLOW = 0x0B; +static constexpr uint8_t FRAME_FLAG_RESERVED_SECTION = 0x0C; +static constexpr uint8_t FRAME_FLAG_TEMP = 0x0D; +static constexpr uint8_t COMMAND_ACK = 0xE5; + +// UART mock with a byte queue for read-side simulation. +class QueuedMockUART : public uart::UARTComponent { + public: + std::deque rx_queue; + std::vector written_data; + + void enqueue(const std::vector &data) { + this->rx_queue.insert(this->rx_queue.end(), data.begin(), data.end()); + } + + void enqueue(std::initializer_list data) { + for (uint8_t byte : data) + this->rx_queue.push_back(byte); + } + + void clear_rx() { this->rx_queue.clear(); } + + bool read_array(uint8_t *data, size_t len) override { + if (this->rx_queue.size() < len) + return false; + for (size_t i = 0; i < len; ++i) { + data[i] = this->rx_queue.front(); + this->rx_queue.pop_front(); + } + return true; + } + + bool peek_byte(uint8_t *data) override { + if (this->rx_queue.empty()) + return false; + *data = this->rx_queue.front(); + return true; + } + + size_t available() override { return this->rx_queue.size(); } + + uart::UARTFlushResult flush() override { return uart::UARTFlushResult::UART_FLUSH_RESULT_SUCCESS; } + + void write_array(const uint8_t *data, size_t len) override { this->written_data.assign(data, data + len); } + + void check_logger_conflict() override {} +#if defined(USE_ESP8266) || defined(USE_ESP32) + void load_settings(bool dump_config) override {} +#endif +}; + +class TestableUFM01 : public UFM01Component { + public: + void set_mock_uart(QueuedMockUART *uart) { this->set_uart_parent(uart); } + + bool process_active_stream() { return this->process_active_stream_(); } + + PassiveReadResult continue_passive_read() { return this->continue_passive_read_(); } + + bool consume_ack() { return this->consume_ack_(); } + + void start_passive_read() { this->start_passive_read_(); } + + void loop_startup() { this->loop_startup_(); } + + OperatingMode operating_mode() const { return this->operating_mode_; } + + StartupPhase startup_phase() const { return this->startup_phase_; } + + int32_t read_index() const { return this->read_index_; } + + size_t passive_index() const { return this->passive_index_; } + + uint32_t last_valid_frame_ms() const { return this->last_valid_frame_ms_; } + + void prepare_passive_read() { + this->passive_index_ = 0; + this->passive_start_ms_ = millis(); + } + + void init_wait_phase() { + this->operating_mode_ = OperatingMode::STARTUP; + this->startup_phase_ = StartupPhase::WAIT; + this->startup_wait_ms_ = 60000; + this->phase_start_ms_ = millis(); + } + + void reset_state() { + this->read_index_ = 0; + this->last_valid_frame_ms_ = 0; + this->passive_index_ = 0; + this->passive_read_pending_ = false; + } +}; + +inline std::array make_active_frame() { + std::array frame{}; + frame[0] = FRAME_START_BYTE_1; + frame[1] = FRAME_START_BYTE_2; + frame[15] = FRAME_FLAG_INSTANT_FLOW; + frame[21] = FRAME_FLAG_RESERVED_SECTION; + frame[24] = FRAME_FLAG_TEMP; + frame[31] = FRAME_STOP_BYTE; + uint8_t sum = 0; + for (size_t i = 0; i < 30; ++i) + sum += frame[i]; + frame[30] = sum; + return frame; +} + +inline std::array make_passive_frame() { + std::array frame{}; + frame[0] = FRAME_START_BYTE_1; + frame[1] = PASSIVE_START_BYTE_2; + frame[9] = FRAME_FLAG_INSTANT_FLOW; + frame[15] = FRAME_FLAG_TEMP; + frame[22] = FRAME_STOP_BYTE; + uint8_t sum = 0; + for (size_t i = 0; i < 21; ++i) + sum += frame[i]; + frame[21] = sum; + return frame; +} + +class UFM01Test : public ::testing::Test { + protected: + void SetUp() override { + this->mock_uart_.clear_rx(); + this->mock_uart_.written_data.clear(); + this->ufm01_.set_mock_uart(&this->mock_uart_); + this->ufm01_.reset_state(); + } + + QueuedMockUART mock_uart_; + TestableUFM01 ufm01_; +}; + +} // namespace esphome::ufm01::testing diff --git a/tests/components/ufm01/ufm01_frame_test.cpp b/tests/components/ufm01/ufm01_frame_test.cpp new file mode 100644 index 0000000000..3b1b148d50 --- /dev/null +++ b/tests/components/ufm01/ufm01_frame_test.cpp @@ -0,0 +1,83 @@ +#include "common.h" + +namespace esphome::ufm01::testing { + +TEST_F(UFM01Test, ValidActiveFrameAccepted) { + auto frame = make_active_frame(); + this->mock_uart_.enqueue(std::vector(frame.begin(), frame.end())); + + EXPECT_TRUE(this->ufm01_.process_active_stream()); + EXPECT_EQ(this->ufm01_.read_index(), 0); + EXPECT_NE(this->ufm01_.last_valid_frame_ms(), 0u); +} + +TEST_F(UFM01Test, GarbagePrefixThenValidActiveFrame) { + this->mock_uart_.enqueue({0x00, 0xFF, 0xAA}); + auto frame = make_active_frame(); + this->mock_uart_.enqueue(std::vector(frame.begin(), frame.end())); + + EXPECT_TRUE(this->ufm01_.process_active_stream()); + EXPECT_EQ(this->ufm01_.read_index(), 0); +} + +TEST_F(UFM01Test, InvalidActiveFrameChecksumRejected) { + auto frame = make_active_frame(); + frame[30] ^= 0xFF; + this->mock_uart_.enqueue(std::vector(frame.begin(), frame.end())); + + EXPECT_FALSE(this->ufm01_.process_active_stream()); + EXPECT_EQ(this->ufm01_.read_index(), 0); + EXPECT_EQ(this->ufm01_.last_valid_frame_ms(), 0u); +} + +TEST_F(UFM01Test, ValidPassiveFrameReadSuccess) { + auto frame = make_passive_frame(); + this->mock_uart_.enqueue(std::vector(frame.begin(), frame.end())); + this->ufm01_.prepare_passive_read(); + + EXPECT_EQ(this->ufm01_.continue_passive_read(), PassiveReadResult::PASSIVE_READ_RESULT_SUCCESS); + EXPECT_EQ(this->ufm01_.passive_index(), PASSIVE_FRAME_SIZE); + EXPECT_NE(this->ufm01_.last_valid_frame_ms(), 0u); +} + +TEST_F(UFM01Test, InvalidPassiveChecksumFails) { + auto frame = make_passive_frame(); + frame[21] ^= 0xFF; + this->mock_uart_.enqueue(std::vector(frame.begin(), frame.end())); + this->ufm01_.prepare_passive_read(); + + EXPECT_EQ(this->ufm01_.continue_passive_read(), PassiveReadResult::PASSIVE_READ_RESULT_FAILURE); + EXPECT_EQ(this->ufm01_.last_valid_frame_ms(), 0u); +} + +TEST_F(UFM01Test, PassiveReadResyncsAfterGarbagePrefix) { + auto frame = make_passive_frame(); + this->mock_uart_.enqueue({0x00, 0x01, 0x02}); + this->mock_uart_.enqueue(std::vector(frame.begin(), frame.end())); + this->ufm01_.prepare_passive_read(); + + EXPECT_EQ(this->ufm01_.continue_passive_read(), PassiveReadResult::PASSIVE_READ_RESULT_SUCCESS); +} + +TEST_F(UFM01Test, PassiveReadResyncsOnSecondStartByte) { + auto frame = make_passive_frame(); + this->mock_uart_.enqueue({FRAME_START_BYTE_1, 0x99}); + this->mock_uart_.enqueue(std::vector(frame.begin(), frame.end())); + this->ufm01_.prepare_passive_read(); + + EXPECT_EQ(this->ufm01_.continue_passive_read(), PassiveReadResult::PASSIVE_READ_RESULT_SUCCESS); +} + +TEST_F(UFM01Test, PassiveReadPendingWhenPartial) { + auto frame = make_passive_frame(); + this->mock_uart_.enqueue(std::vector(frame.begin(), frame.begin() + 10)); + this->ufm01_.prepare_passive_read(); + + EXPECT_EQ(this->ufm01_.continue_passive_read(), PassiveReadResult::PASSIVE_READ_RESULT_PENDING); + EXPECT_LT(this->ufm01_.passive_index(), PASSIVE_FRAME_SIZE); + + this->mock_uart_.enqueue(std::vector(frame.begin() + 10, frame.end())); + EXPECT_EQ(this->ufm01_.continue_passive_read(), PassiveReadResult::PASSIVE_READ_RESULT_SUCCESS); +} + +} // namespace esphome::ufm01::testing diff --git a/tests/components/ufm01/ufm01_startup_test.cpp b/tests/components/ufm01/ufm01_startup_test.cpp new file mode 100644 index 0000000000..6b7feb0f1b --- /dev/null +++ b/tests/components/ufm01/ufm01_startup_test.cpp @@ -0,0 +1,43 @@ +#include "common.h" + +#include "esphome/core/component.h" + +namespace esphome::ufm01::testing { + +TEST(UFM01SetupPriority, IsLate) { + TestableUFM01 ufm01; + EXPECT_EQ(ufm01.get_setup_priority(), setup_priority::LATE); +} + +TEST_F(UFM01Test, ConsumeAckFindsByteAmongGarbage) { + this->mock_uart_.enqueue({0x00, 0x01, COMMAND_ACK, 0x02}); + + EXPECT_TRUE(this->ufm01_.consume_ack()); + EXPECT_EQ(this->mock_uart_.available(), 1u); +} + +TEST_F(UFM01Test, ConsumeAckReturnsFalseWhenEmpty) { EXPECT_FALSE(this->ufm01_.consume_ack()); } + +TEST_F(UFM01Test, StartupWaitDetectsActiveStream) { + auto frame = make_active_frame(); + this->mock_uart_.enqueue(std::vector(frame.begin(), frame.end())); + this->ufm01_.init_wait_phase(); + + this->ufm01_.loop_startup(); + + EXPECT_EQ(this->ufm01_.operating_mode(), OperatingMode::ACTIVE_STREAM); + EXPECT_EQ(this->ufm01_.startup_phase(), StartupPhase::WAIT); +} + +TEST_F(UFM01Test, StartPassiveReadSendsCommand) { + this->ufm01_.start_passive_read(); + + ASSERT_EQ(this->mock_uart_.written_data.size(), 7u); + EXPECT_EQ(this->mock_uart_.written_data[0], 0xFE); + EXPECT_EQ(this->mock_uart_.written_data[1], 0xFE); + EXPECT_EQ(this->mock_uart_.written_data[2], 0x11); + EXPECT_EQ(this->mock_uart_.written_data[3], 0x5B); + EXPECT_EQ(this->mock_uart_.written_data[6], FRAME_STOP_BYTE); +} + +} // namespace esphome::ufm01::testing diff --git a/tests/components/web_server/test-basicauth.esp8266-ard.yaml b/tests/components/web_server/test-basicauth.esp8266-ard.yaml new file mode 100644 index 0000000000..6a01180892 --- /dev/null +++ b/tests/components/web_server/test-basicauth.esp8266-ard.yaml @@ -0,0 +1,8 @@ +packages: + web_server: !include common_v2.yaml + +web_server: + auth: + username: admin + password: AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA + type: basic diff --git a/tests/components/web_server/test.rp2040-ard.yaml b/tests/components/web_server/test.rp2040-ard.yaml index e4d50d7776..6a01180892 100644 --- a/tests/components/web_server/test.rp2040-ard.yaml +++ b/tests/components/web_server/test.rp2040-ard.yaml @@ -4,5 +4,5 @@ packages: web_server: auth: username: admin - password: password + password: AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA type: basic diff --git a/tests/integration/fixtures/uart_mock_modbus_client_read_write.yaml b/tests/integration/fixtures/uart_mock_modbus_client_read_write.yaml new file mode 100644 index 0000000000..1f89889c95 --- /dev/null +++ b/tests/integration/fixtures/uart_mock_modbus_client_read_write.yaml @@ -0,0 +1,111 @@ +esphome: + name: uart-mock-modbus-cli-rw + +host: +api: +logger: + level: VERBOSE + +external_components: + - source: + type: local + path: EXTERNAL_COMPONENT_PATH + +# Dummy uart entry to satisfy modbus's DEPENDENCIES = ["uart"] +# The actual UART bus used is the uart_mock component below +uart: + baud_rate: 115200 + port: /dev/null + +# Two virtual buses looped back to each other: the client's transmissions reach the server and the +# server's replies reach the client. auto_start so forwarding is active before the button fires. +uart_mock: + - id: virtual_uart_server + baud_rate: 9600 + auto_start: true + debug: + on_tx: + - then: + - uart_mock.inject_rx: + id: virtual_uart_client + data: !lambda return data; + - id: virtual_uart_client + baud_rate: 9600 + auto_start: true + debug: + on_tx: + - then: + - uart_mock.inject_rx: + id: virtual_uart_server + data: !lambda return data; + +globals: + - id: stored_1 + type: uint16_t + initial_value: "0" + +modbus: + - uart_id: virtual_uart_server + id: virtual_modbus_server + role: server + - uart_id: virtual_uart_client + id: virtual_modbus_client + role: client + turnaround_time: 10ms + +modbus_server: + - address: 1 + modbus_id: virtual_modbus_server + registers: + # Writable + readable register: the read publishes what it returns, so the test can confirm the + # write half of the 0x17 ran before the read half (Modbus 6.17). + - address: 0x01 + value_type: U_WORD + read_lambda: |- + id(srv_read_1).publish_state(id(stored_1)); + return id(stored_1); + write_lambda: |- + id(stored_1) = x; + id(srv_write_1).publish_state(x); + return true; + # Read-only register, returned together with 0x01 by the 2-register read half. + - address: 0x02 + value_type: U_WORD + read_lambda: return 0x00AA; + +sensor: + # Server-side observations. + - platform: template + name: "srv_write_1" + id: srv_write_1 + - platform: template + name: "srv_read_1" + id: srv_read_1 + # Client-side read-back: the values the client's on_response received. + - platform: template + name: "client_read_0" + id: client_read_0 + - platform: template + name: "client_read_1" + id: client_read_1 + +button: + - platform: template + name: "Start Scenario" + id: start_scenario_btn + on_press: + # FC 0x17: write reg 0x0001 = 0x1234, then read regs 0x0001..0x0002 back in the same transaction. + - modbus_client.read_write_multiple_registers: + address: 0x01 + read_address: 0x0001 + read_count: 2 + write_address: 0x0001 + values: [0x1234] + on_response: + then: + - lambda: |- + // values is the read-back block: reg 0x0001 (must be the just-written 0x1234) and reg 0x0002. + if (values.size() >= 2) { + id(client_read_0).publish_state(values[0]); + id(client_read_1).publish_state(values[1]); + } diff --git a/tests/integration/test_uart_mock_modbus.py b/tests/integration/test_uart_mock_modbus.py index ca0041cc5b..1994d02c34 100644 --- a/tests/integration/test_uart_mock_modbus.py +++ b/tests/integration/test_uart_mock_modbus.py @@ -756,3 +756,38 @@ async def test_uart_mock_modbus_fairness( f"controllers did not get a fair share of the bus: " f"controller 1 issued {count_1}, controller 2 issued {count_2}" ) + + +@pytest.mark.asyncio +async def test_uart_mock_modbus_client_read_write( + yaml_config: str, + run_compiled: RunCompiledFunction, + api_client_connected: APIClientConnectedFactory, +) -> None: + """A modbus_client.read_write_multiple_registers action (FC 0x17) drives a server end to end. + + The client writes reg 0x0001 = 0x1234 and reads regs 0x0001..0x0002 in one transaction; the server + applies the write first (Modbus 6.17). The test confirms both ends: the server's write_lambda ran + (srv_write_1) and the read half came back to the client's on_response (client_read_0 = the + just-written 0x1234, client_read_1 = the read-only 0x00AA). + """ + line_callback, error_log_lines, warning_log_lines = _make_modbus_line_callback() + + tracker = SensorTracker( + ["srv_write_1", "srv_read_1", "client_read_0", "client_read_1"] + ) + futures = tracker.expect_all( + { + "srv_write_1": 4660, # server wrote 0x1234 to reg 0x0001 + "client_read_0": 4660, # client read reg 0x0001 back as the just-written 0x1234 + "client_read_1": 170, # client read reg 0x0002 (0x00AA) in the same request + } + ) + + async with ( + run_compiled(yaml_config, line_callback=line_callback), + api_client_connected() as client, + ): + await tracker.setup_and_start_scenario(client) + await tracker.await_all(futures) + _assert_no_modbus_errors(error_log_lines, warning_log_lines) diff --git a/tests/unit_tests/test_config_validation.py b/tests/unit_tests/test_config_validation.py index 4a4e37e5c4..7627ef9273 100644 --- a/tests/unit_tests/test_config_validation.py +++ b/tests/unit_tests/test_config_validation.py @@ -932,7 +932,7 @@ def test_string_no_slash__slash_replaced_with_warning( actual = cv.string_no_slash(value) assert actual == expected assert "reserved as a URL path separator" in caplog.text - assert "will become an error in ESPHome 2026.7.0" in caplog.text + assert "will become an error in ESPHome 2027.7.0" in caplog.text def test_string_no_slash__long_string_allowed() -> None: