From 859851cfb508b34aa7e9f69bb007f9aa932e4fee Mon Sep 17 00:00:00 2001 From: Claas Date: Sun, 23 Aug 2026 13:18:32 +0200 Subject: [PATCH] Name the modules added for sensor polling in the singular The `fan_sensors` crate is now `fan_sensor` and is re-exported as `crate::fan::sensor`, `input_registers` is `input_register`, and the modbus `read_input_registers` module is `read_input_register`. The topic crate's per-fan `sensors` module is `sensor`. `holding_registers` came along with them. It predates this work, but leaving it plural next to a singular `input_register` would have made the inconsistency look deliberate. Only module names changed. The `ReadInputRegisters` type and the `Client::read_input_registers` method stay plural, because both genuinely handle a range rather than one register, and the MQTT topic segments stay `sensors` because they are the wire format rather than a name in the source. The generated discovery payload is byte for byte what it was. Co-Authored-By: Claude Opus 5 --- CLAUDE.md | 4 +-- Cargo.lock | 4 +-- Cargo.toml | 2 +- fan-controller/Cargo.toml | 2 +- fan-controller/TODO.md | 4 +-- fan-controller/build.rs | 26 +++++++++---------- fan-controller/src/fan/mod.rs | 14 +++++----- fan-controller/src/main.rs | 22 ++++++++-------- fan-controller/src/modbus/client.rs | 4 +-- fan-controller/src/modbus/function/mod.rs | 4 +-- ...ut_registers.rs => read_input_register.rs} | 0 {fan_sensors => fan_sensor}/Cargo.toml | 2 +- {fan_sensors => fan_sensor}/src/lib.rs | 0 topic/src/lib.rs | 4 +-- 14 files changed, 46 insertions(+), 46 deletions(-) rename fan-controller/src/modbus/function/{read_input_registers.rs => read_input_register.rs} (100%) rename {fan_sensors => fan_sensor}/Cargo.toml (91%) rename {fan_sensors => fan_sensor}/src/lib.rs (100%) diff --git a/CLAUDE.md b/CLAUDE.md index 5f7ef9b..32c9018 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -64,7 +64,7 @@ Testing reality below. | `mqtt` | `no_std` | Protocol-level MQTT types shared between firmware and build script. Feature-gated `defmt` / `serde` so the same types work on device and on host. | | `topic` | `no_std` | The single source of truth for Home Assistant MQTT topic strings, composed at compile time with `const_format`. Used by both the firmware and `build.rs`. | | `set_point` | `no_std` | The `SetPoint` newtype and its bounds, parsing and formatting. Its own crate purely so it can be tested on the host; re-exported by the firmware as `crate::fan::set_point`. Feature-gated `defmt`. | -| `fan_sensors` | `no_std` | Decoding what a fan reports about itself — actual speed, both temperatures, power, energy — from its input registers, plus the JSON payload Home Assistant reads. Owns the register addresses and the layout of the two runs that are read. Its own crate for the same reason as `set_point`; re-exported as `crate::fan::sensors`. Feature-gated `defmt`. | +| `fan_sensor` | `no_std` | Decoding what a fan reports about itself — actual speed, both temperatures, power, energy — from its input registers, plus the JSON payload Home Assistant reads. Owns the register addresses and the layout of the two runs that are read. Its own crate for the same reason as `set_point`; re-exported as `crate::fan::sensor`. Feature-gated `defmt`. | | `home_assistant_discovery` | host | Serde model of the Home Assistant MQTT discovery payload. Build-dependency only. `components` is a `BTreeMap` so the generated payload is byte-stable across builds. | | `debug-listener` | host | Reads the RS-485/Modbus line off a USB serial adapter to inspect fan traffic. The port path is hardcoded in `src/main.rs`. | @@ -152,7 +152,7 @@ cd home_assistant_discovery && cargo test ``` ```bash -cd fan_sensors && cargo test +cd fan_sensor && cargo test ``` That is also the way to make firmware logic testable at all: move it into its own `no_std` crate diff --git a/Cargo.lock b/Cargo.lock index adf5033..0fafaeb 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1018,7 +1018,7 @@ dependencies = [ "embassy-time", "embedded-io-async", "embedded-nal-async", - "fan_sensors", + "fan_sensor", "heapless 0.8.0", "home_assistant_discovery", "mqtt", @@ -1035,7 +1035,7 @@ dependencies = [ ] [[package]] -name = "fan_sensors" +name = "fan_sensor" version = "0.1.0" dependencies = [ "defmt 1.0.1", diff --git a/Cargo.toml b/Cargo.toml index 74a09e8..0eb8980 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -2,7 +2,7 @@ members = [ "debug-listener", "fan-controller", - "fan_sensors", + "fan_sensor", "home_assistant_discovery", "mqtt", "set_point", diff --git a/fan-controller/Cargo.toml b/fan-controller/Cargo.toml index cc9c076..b8b8805 100644 --- a/fan-controller/Cargo.toml +++ b/fan-controller/Cargo.toml @@ -39,7 +39,7 @@ embassy-sync = { git = "https://github.com/embassy-rs/embassy.git", package = "e embassy-time = { version = "0.3.1", features = ["defmt"] } embedded-io-async = "0.6.1" embedded-nal-async = "0.7.1" -fan_sensors = { version = "0.1.0", path = "../fan_sensors", features = ["defmt"] } +fan_sensor = { version = "0.1.0", path = "../fan_sensor", features = ["defmt"] } heapless = "0.8.0" mqtt = { version = "0.1.0", path = "../mqtt", features = ["defmt"] } nb = "1.1.0" diff --git a/fan-controller/TODO.md b/fan-controller/TODO.md index a95fced..8056ffe 100644 --- a/fan-controller/TODO.md +++ b/fan-controller/TODO.md @@ -188,7 +188,7 @@ which means pulling the bus rather than anything Home Assistant can ask for. ### Done since — sensor polling -**Poll what the fans measure about themselves** — done, `fan_sensors/`, `src/modbus/`, `src/main.rs` +**Poll what the fans measure about themselves** — done, `fan_sensor/`, `src/modbus/`, `src/main.rs` This is the "Read temperature sensors" item from `README.md`, done wider than it was written: the fans report an actual speed, a motor temperature, an electronics temperature, a current power draw @@ -208,7 +208,7 @@ dropped rather than retried: the next poll carries fresher values than a retry w that has stopped answering does not hold the Modbus mutex through a run of timeouts while a speed change waits behind it. -Decoding lives in the `fan_sensors` crate, following the `set_point` pattern, so the rules it has — +Decoding lives in the `fan_sensor` crate, following the `set_point` pattern, so the rules it has — a speed that is a fraction of the fan's configured maximum, temperatures that are signed, an energy counter spanning two registers — are tested on the host. The fan's maximum speed (`D119`) is read once and cached; until it is known the reading reports the speed as `null`, which Home Assistant diff --git a/fan-controller/build.rs b/fan-controller/build.rs index d5dbd92..ad7c07e 100644 --- a/fan-controller/build.rs +++ b/fan-controller/build.rs @@ -127,7 +127,7 @@ struct SensorIdentifiers { /// /// Every one of them reads the same topic and picks its value out of the JSON object published /// there, so a poll costs one publish rather than five. The keys the templates use are the field -/// names `fan_sensors::Reading` serializes, which is the one place they have to agree. +/// names `fan_sensor::Reading` serializes, which is the one place they have to agree. /// /// Built per fan rather than written out twice, because only the identifiers and the name differ fn fan_sensor_components( @@ -240,26 +240,26 @@ fn components() -> BTreeMap { components.extend(fan_sensor_components( "Fan 1", SensorIdentifiers { - state: topic::fan_controller::fan_1::sensors::STATE, - speed: topic::fan_controller::fan_1::sensors::SPEED, - motor_temperature: topic::fan_controller::fan_1::sensors::MOTOR_TEMPERATURE, + state: topic::fan_controller::fan_1::sensor::STATE, + speed: topic::fan_controller::fan_1::sensor::SPEED, + motor_temperature: topic::fan_controller::fan_1::sensor::MOTOR_TEMPERATURE, electronics_temperature: - topic::fan_controller::fan_1::sensors::ELECTRONICS_TEMPERATURE, - power: topic::fan_controller::fan_1::sensors::POWER, - energy: topic::fan_controller::fan_1::sensors::ENERGY, + topic::fan_controller::fan_1::sensor::ELECTRONICS_TEMPERATURE, + power: topic::fan_controller::fan_1::sensor::POWER, + energy: topic::fan_controller::fan_1::sensor::ENERGY, }, )); components.extend(fan_sensor_components( "Fan 2", SensorIdentifiers { - state: topic::fan_controller::fan_2::sensors::STATE, - speed: topic::fan_controller::fan_2::sensors::SPEED, - motor_temperature: topic::fan_controller::fan_2::sensors::MOTOR_TEMPERATURE, + state: topic::fan_controller::fan_2::sensor::STATE, + speed: topic::fan_controller::fan_2::sensor::SPEED, + motor_temperature: topic::fan_controller::fan_2::sensor::MOTOR_TEMPERATURE, electronics_temperature: - topic::fan_controller::fan_2::sensors::ELECTRONICS_TEMPERATURE, - power: topic::fan_controller::fan_2::sensors::POWER, - energy: topic::fan_controller::fan_2::sensors::ENERGY, + topic::fan_controller::fan_2::sensor::ELECTRONICS_TEMPERATURE, + power: topic::fan_controller::fan_2::sensor::POWER, + energy: topic::fan_controller::fan_2::sensor::ENERGY, }, )); diff --git a/fan-controller/src/fan/mod.rs b/fan-controller/src/fan/mod.rs index 15ae16d..142caf9 100644 --- a/fan-controller/src/fan/mod.rs +++ b/fan-controller/src/fan/mod.rs @@ -7,7 +7,7 @@ pub(crate) use ::set_point; /// Decoding what the fan reports about itself, in its own crate for the same reason as /// [`set_point`] and re-exported here for the same one -pub(crate) use ::fan_sensors as sensors; +pub(crate) use ::fan_sensor as sensor; use embassy_rp::uart::{self, DataBits, Parity, StopBits}; @@ -55,7 +55,7 @@ pub(crate) mod address { pub(crate) const FAN_2: modbus::device::Address = modbus::device::Address::new(0x03); } -pub(super) mod holding_registers { +pub(super) mod holding_register { use crate::modbus; pub(crate) const REFERENCE_SET_POINT: modbus::register::Address = @@ -64,23 +64,23 @@ pub(super) mod holding_registers { /// The speed the fan is configured for, which every speed it reports and accepts is a fraction /// of. Only changes when the fan is reconfigured, so it is read once rather than on every poll pub(crate) const MAXIMUM_SPEED: modbus::register::Address = - modbus::register::Address::new(super::sensors::MAXIMUM_SPEED_REGISTER); + modbus::register::Address::new(super::sensor::MAXIMUM_SPEED_REGISTER); } /// Where the fan reports what it measures about itself. Read only, and read as two runs rather /// than register by register because a range costs the same round trip as one register. -/// The addresses and the layout of each run belong to the [`sensors`] crate, which is what decodes +/// The addresses and the layout of each run belong to the [`sensor`] crate, which is what decodes /// them; these only wrap them in the address type the modbus client asks for -pub(super) mod input_registers { +pub(super) mod input_register { use crate::modbus; /// The run holding the actual speed and both temperatures pub(crate) const STATUS: modbus::register::Address = - modbus::register::Address::new(super::sensors::STATUS_START); + modbus::register::Address::new(super::sensor::STATUS_START); /// The run holding the current power draw and the energy counter pub(crate) const ENERGY: modbus::register::Address = - modbus::register::Address::new(super::sensors::ENERGY_START); + modbus::register::Address::new(super::sensor::ENERGY_START); } #[derive(Clone, Copy)] diff --git a/fan-controller/src/main.rs b/fan-controller/src/main.rs index 046cf48..662ddc8 100644 --- a/fan-controller/src/main.rs +++ b/fan-controller/src/main.rs @@ -551,7 +551,7 @@ enum OutgoingPublish { /// channel UpdateSensors { fan: Fan, - payload: heapless::String<{ fan::sensors::JSON_CAPACITY }>, + payload: heapless::String<{ fan::sensor::JSON_CAPACITY }>, }, } @@ -576,10 +576,10 @@ impl Publish for OutgoingPublish { payload: _, } => topic::fan_controller::fan_2::state::STATE, OutgoingPublish::UpdateSensors { fan: Fan::One, .. } => { - topic::fan_controller::fan_1::sensors::STATE + topic::fan_controller::fan_1::sensor::STATE } OutgoingPublish::UpdateSensors { fan: Fan::Two, .. } => { - topic::fan_controller::fan_2::sensors::STATE + topic::fan_controller::fan_2::sensor::STATE } } } @@ -764,7 +764,7 @@ async fn read_set_point( ) -> Option { let function = modbus::function::ReadHoldingRegister::new( fan_address, - fan::holding_registers::REFERENCE_SET_POINT, + fan::holding_register::REFERENCE_SET_POINT, ); for attempt in 1..=MAX_ATTEMPTS { @@ -883,7 +883,7 @@ async fn fan_control_routine( let function = modbus::function::WriteHoldingRegister::new( fan_address, - fan::holding_registers::REFERENCE_SET_POINT, + fan::holding_register::REFERENCE_SET_POINT, *set_point, ); @@ -996,7 +996,7 @@ async fn sensor_routine( if maximum_speed.is_none() { let function = modbus::function::ReadHoldingRegister::new( fan_address, - fan::holding_registers::MAXIMUM_SPEED, + fan::holding_register::MAXIMUM_SPEED, ); match modbus_mutex @@ -1019,11 +1019,11 @@ async fn sensor_routine( } let status_request = modbus::function::ReadInputRegisters::< - { fan::sensors::STATUS_LENGTH }, - >::new(fan_address, fan::input_registers::STATUS); + { fan::sensor::STATUS_LENGTH }, + >::new(fan_address, fan::input_register::STATUS); let energy_request = modbus::function::ReadInputRegisters::< - { fan::sensors::ENERGY_LENGTH }, - >::new(fan_address, fan::input_registers::ENERGY); + { fan::sensor::ENERGY_LENGTH }, + >::new(fan_address, fan::input_register::ENERGY); // Both runs are read under one lock so the five values describe the same moment. It costs // a speed change at most the two transactions rather than one, which is still well under a @@ -1031,7 +1031,7 @@ async fn sensor_routine( let mut client = modbus_mutex.lock().await; let reading = match client.read_input_registers(&status_request).await { Ok(status) => match client.read_input_registers(&energy_request).await { - Ok(energy) => Some(fan::sensors::decode(&status, &energy, maximum_speed)), + Ok(energy) => Some(fan::sensor::decode(&status, &energy, maximum_speed)), Err(error) => { warn!( "{} Failed to read the energy registers: {:?}", diff --git a/fan-controller/src/modbus/client.rs b/fan-controller/src/modbus/client.rs index 26043e7..dc70dcc 100644 --- a/fan-controller/src/modbus/client.rs +++ b/fan-controller/src/modbus/client.rs @@ -11,7 +11,7 @@ use embedded_io_async::{Read, ReadExactError, Write}; use crate::{ configuration, modbus::function::{ - ReadHoldingRegister, ReadInputRegisters, WriteHoldingRegister, code, read_input_registers, + ReadHoldingRegister, ReadInputRegisters, WriteHoldingRegister, code, read_input_register, }, }; @@ -197,7 +197,7 @@ const READ_OVERHEAD_LENGTH: usize = HEADER_LENGTH + 1 + 2; /// them is read into. An array cannot be sized from a const generic on stable, so the buffer is /// sized for the longest run the fan will answer and only the part that was asked for is used const MAX_INPUT_REGISTERS_RESPONSE_LENGTH: usize = - READ_OVERHEAD_LENGTH + 2 * read_input_registers::MAX_COUNT; + READ_OVERHEAD_LENGTH + 2 * read_input_register::MAX_COUNT; /// An exception response replaces the register and value of the echo with a single exception code const EXCEPTION_RESPONSE_LENGTH: usize = 5; diff --git a/fan-controller/src/modbus/function/mod.rs b/fan-controller/src/modbus/function/mod.rs index e31d48c..43f5a55 100644 --- a/fan-controller/src/modbus/function/mod.rs +++ b/fan-controller/src/modbus/function/mod.rs @@ -1,8 +1,8 @@ pub(super) mod code; pub(crate) mod read_holding_register; -pub(crate) mod read_input_registers; +pub(crate) mod read_input_register; pub(crate) mod write_holding_register; pub(crate) use read_holding_register::ReadHoldingRegister; -pub(crate) use read_input_registers::ReadInputRegisters; +pub(crate) use read_input_register::ReadInputRegisters; pub(crate) use write_holding_register::WriteHoldingRegister; diff --git a/fan-controller/src/modbus/function/read_input_registers.rs b/fan-controller/src/modbus/function/read_input_register.rs similarity index 100% rename from fan-controller/src/modbus/function/read_input_registers.rs rename to fan-controller/src/modbus/function/read_input_register.rs diff --git a/fan_sensors/Cargo.toml b/fan_sensor/Cargo.toml similarity index 91% rename from fan_sensors/Cargo.toml rename to fan_sensor/Cargo.toml index 676d4ec..1efeac1 100644 --- a/fan_sensors/Cargo.toml +++ b/fan_sensor/Cargo.toml @@ -1,5 +1,5 @@ [package] -name = "fan_sensors" +name = "fan_sensor" version = "0.1.0" edition = "2024" diff --git a/fan_sensors/src/lib.rs b/fan_sensor/src/lib.rs similarity index 100% rename from fan_sensors/src/lib.rs rename to fan_sensor/src/lib.rs diff --git a/topic/src/lib.rs b/topic/src/lib.rs index 06bf409..cea637d 100644 --- a/topic/src/lib.rs +++ b/topic/src/lib.rs @@ -45,7 +45,7 @@ pub mod fan_controller { /// All five sensor values a fan reports arrive as one JSON object on this topic, so a poll /// costs a single publish and Home Assistant picks each value out with a value template - pub mod sensors { + pub mod sensor { use super::UNIQUE_ID; use const_format::formatcp; @@ -91,7 +91,7 @@ pub mod fan_controller { /// All five sensor values a fan reports arrive as one JSON object on this topic, so a poll /// costs a single publish and Home Assistant picks each value out with a value template - pub mod sensors { + pub mod sensor { use super::UNIQUE_ID; use const_format::formatcp; -- 2.51.2