diff --git a/fan-controller/README.md b/fan-controller/README.md index 593d244..aa11d00 100644 --- a/fan-controller/README.md +++ b/fan-controller/README.md @@ -14,6 +14,7 @@ install [probe.rs](https://probe.rs) `cargo run` - [ ] Read temperature sensors - [ ] Switch to only using refactored send for modbus - [ ] [Reboot](https://github.com/embassy-rs/embassy/blob/f8685560531fcecb2f4327a490ec4df4f2b190f6/examples/rp/src/bin/rtc.rs#L50) on error after retries or connection lost. Or try reconnect in background. +- [ ] Try out bundling all channels in a sort of event bus like actor model ## Aspirational TODOs diff --git a/fan-controller/src/async_callback.rs b/fan-controller/src/async_callback.rs deleted file mode 100644 index 014a734..0000000 --- a/fan-controller/src/async_callback.rs +++ /dev/null @@ -1,19 +0,0 @@ -use core::future::Future; - -/// Workaround for calling async function callback with lifetime parameter -/// Source:https://www.reddit.com/r/rust/comments/hey4oa/comment/fvv1zql/ -pub trait AsyncCallback<'a, T> { - type Output: 'a + Future; - fn call(&self, argument: &'a T) -> Self::Output; -} - -impl<'a, R: 'a, F, T: 'a> AsyncCallback<'a, T> for F -where - F: Fn(&'a T) -> R, - R: Future + 'a, -{ - type Output = R; - fn call(&self, argument: &'a T) -> Self::Output { - self(argument) - } -} diff --git a/fan-controller/src/configuration.rs b/fan-controller/src/configuration.rs index 1ee298b..144e057 100644 --- a/fan-controller/src/configuration.rs +++ b/fan-controller/src/configuration.rs @@ -1,5 +1,4 @@ use crate::{MqttBrokerConfiguration, task::MqttBrokerCredentials}; -use embassy_net::IpAddress; use embassy_time::Duration; /// Thanks! @@ -35,11 +34,6 @@ pub(crate) const MQTT_BROKER_ADDRESS: &str = env!("FAN_CONTROL_MQTT_BROKER_ADDRE //TODO make configurable pub(crate) const MQTT_BROKER_PORT: u16 = parse_u16(env!("FAN_CONTROL_MQTT_BROKER_PORT")); -//TODO make configurable -/// The broker IP address can be configured manually. It will be used instead of the [MQTT_BROKER_ADDRESS] -/// if it is set as it does not require DNS resolution. -pub(crate) const MQTT_BROKER_IP_ADDRESS: Option = None; - pub(crate) const MQTT_BROKER_CREDENTIALS: MqttBrokerCredentials = MqttBrokerCredentials { username: env!("FAN_CONTROL_MQTT_BROKER_USERNAME"), password: env!("FAN_CONTROL_MQTT_BROKER_PASSWORD").as_bytes(), diff --git a/fan-controller/src/debounce.rs b/fan-controller/src/debounce.rs index 2ee2d41..8a0a114 100644 --- a/fan-controller/src/debounce.rs +++ b/fan-controller/src/debounce.rs @@ -1,5 +1,5 @@ -use embassy_rp::gpio::{Input, Level, Pin}; -use embassy_time::{Duration, TimeoutError, Timer, with_timeout}; +use embassy_rp::gpio::{Input, Pin}; +use embassy_time::{Duration, TimeoutError, with_timeout}; /// Debouncer based on [Embassy debounce example](https://github.com/embassy-rs/embassy/blob/8d8cd78f634b2f435e3a997f7f8f3ac0b8ca300c/examples/rp/src/bin/debounce.rs) /// (Licensed MIT/Apache-2.0) @@ -15,23 +15,6 @@ impl<'a, T: Pin> Debouncer<'a, T> { Self { input, debounce } } - pub async fn debounce(&mut self) -> Level { - loop { - // Up - // 2nd round stil down - let l1 = self.input.get_level(); - - self.input.wait_for_any_edge().await; - - Timer::after(self.debounce).await; - - let l2 = self.input.get_level(); - if l1 != l2 { - break l2; - } - } - } - pub async fn debounce_falling_edge(&mut self) { self.input.wait_for_falling_edge().await; loop { diff --git a/fan-controller/src/event_bus.rs b/fan-controller/src/event_bus.rs deleted file mode 100644 index 062fe9b..0000000 --- a/fan-controller/src/event_bus.rs +++ /dev/null @@ -1,7 +0,0 @@ -/// An idea to create a logical event bus through which all events of the system can be sent -/// This can not be implemented as a single channel because that would probably lead to too much pressure -/// and messages getting dropped when there are events for which we need to ensure delivery. -/// For example when there is an event to update 2 fans, we need to ensure that both fans receive the message. -/// To solve this there are two signals used for both fans. But they are opaque to the outside. -/// Using a single event bus should help logically bundle all channels together and prevents global static state to spread across the code base. -struct EventBus {} diff --git a/fan-controller/src/fan/mod.rs b/fan-controller/src/fan/mod.rs index 1b9a633..35034c2 100644 --- a/fan-controller/src/fan/mod.rs +++ b/fan-controller/src/fan/mod.rs @@ -3,7 +3,6 @@ pub(crate) mod set_point; -use defmt::Format; use embassy_rp::uart::{self, DataBits, Parity, StopBits}; pub(crate) const BAUD_RATE: u32 = 19_200; @@ -26,41 +25,22 @@ pub(crate) mod user_setting { /// Max speed 64000 / 3.3 pub(crate) const LOW: SetPoint = match SetPoint::new(19_393) { Ok(setting) => setting, - Err(error) => panic!("Invalid value"), + Err(_error) => panic!("Invalid value"), }; /// Max speed 64000 / 2.4 pub(crate) const MEDIUM: SetPoint = match SetPoint::new(26_666) { Ok(setting) => setting, - Err(error) => panic!("Invalid value"), + Err(_error) => panic!("Invalid value"), }; /// Max speed 50% /// Not set to full speed to not wear out the fans pub(crate) const HIGH: SetPoint = match SetPoint::new(set_point::MAX / 2) { Ok(setting) => setting, - Err(error) => panic!("Invalid value"), + Err(_error) => panic!("Invalid value"), }; } -#[derive(Default, Format, Debug)] -pub(crate) enum State { - #[default] - Off, - Low, - Medium, - High, -} -impl State { - pub(crate) const fn next(&self) -> Self { - match self { - Self::Off => Self::Low, - Self::Low => Self::Medium, - Self::Medium => Self::High, - Self::High => Self::Off, - } - } -} - pub(crate) mod address { use crate::modbus; @@ -76,35 +56,7 @@ pub(super) mod holding_registers { modbus::register::Address::new(0xd001_u16); } -pub(crate) mod input_registers { - use crate::modbus; - - pub(crate) const TEMPERATURE_SENSOR_1: modbus::register::Address = - modbus::register::Address::new(0xd02e_u16); - pub(crate) const HUMIDITY_SENSOR_1: modbus::register::Address = - modbus::register::Address::new(0xd02f_u16); - pub(crate) const TEMPERATURE_SENSOR_2: modbus::register::Address = - modbus::register::Address::new(0xd030_u16); - pub(crate) const HUMIDITY_SENSOR_2: modbus::register::Address = - modbus::register::Address::new(0xd031_u16); -} - pub(crate) enum Fan { One, Two, } - -pub(crate) struct FanResponse { - data: [u8; N], - length: usize, -} - -impl FanResponse { - pub(crate) fn new(data: [u8; N], length: usize) -> Self { - Self { data, length } - } - - pub(crate) fn as_slice(&self) -> &[u8] { - &self.data[..self.length] - } -} diff --git a/fan-controller/src/fan/set_point.rs b/fan-controller/src/fan/set_point.rs index 856d860..c5f4776 100644 --- a/fan-controller/src/fan/set_point.rs +++ b/fan-controller/src/fan/set_point.rs @@ -14,17 +14,7 @@ pub(crate) struct SetPointOutOfBoundsError; impl SetPoint { pub(crate) const ZERO: Self = match Self::new(0) { Ok(setting) => setting, - Err(error) => panic!("Invalid value. This should not be reachable."), - }; - - pub(crate) const MAX: Self = match Self::new(MAX) { - Ok(setting) => setting, - Err(error) => panic!("Invalid value. This should not be reachable."), - }; - - pub(crate) const MIN: Self = match Self::new(0) { - Ok(setting) => setting, - Err(error) => panic!("Invalid value. This should not be reachable."), + Err(_error) => panic!("Invalid value. This should not be reachable."), }; pub(crate) const fn new(set_point: u16) -> Result { @@ -35,10 +25,6 @@ impl SetPoint { Ok(Self(set_point)) } - const fn get(&self) -> u16 { - self.0 - } - /// This should always succeed pub(crate) fn to_string(&self) -> heapless::String<5> { heapless::String::<5>::try_from(self.0) @@ -55,15 +41,21 @@ impl Deref for SetPoint { } pub(crate) enum ParseSetPointError { - ParseInt(core::num::ParseIntError), + ParseInt, SettingOutOfBounds(SetPointOutOfBoundsError), } +impl From for ParseSetPointError { + fn from(_error: core::num::ParseIntError) -> Self { + ParseSetPointError::ParseInt + } +} + impl FromStr for SetPoint { type Err = ParseSetPointError; fn from_str(s: &str) -> Result { - let set_point = s.parse().map_err(ParseSetPointError::ParseInt)?; + let set_point = s.parse()?; Self::new(set_point).map_err(ParseSetPointError::SettingOutOfBounds) } } diff --git a/fan-controller/src/main.rs b/fan-controller/src/main.rs index 0f4e8dd..a8e25b0 100644 --- a/fan-controller/src/main.rs +++ b/fan-controller/src/main.rs @@ -1,6 +1,8 @@ #![no_std] #![no_main] +use core::str; + use cyw43::{Control, NetDriver}; use cyw43_pio::PioSpi; use debounce::Debouncer; @@ -33,10 +35,8 @@ use crate::mqtt::packet::ping_request::PingRequest; use crate::mqtt::packet::publish; use crate::task::{MqttBrokerConfiguration, Publish, set_up_network_stack}; -mod async_callback; mod configuration; mod debounce; -mod event_bus; mod fan; mod modbus; mod mqtt; @@ -381,13 +381,19 @@ enum IncomingPublish { enum FromPublishError { // Invalid fan command - InvalidStringPayload(core::str::Utf8Error), + InvalidStringPayload, ParseSetPoint(ParseSetPointError), InvalidSetStateCommandPayload, UnknownTopic, } +impl From for FromPublishError { + fn from(_: str::Utf8Error) -> Self { + FromPublishError::InvalidStringPayload + } +} + impl TryFrom> for IncomingPublish { type Error = FromPublishError; @@ -402,11 +408,10 @@ impl TryFrom> for IncomingPublish { target: Fan::One, command: FanCommand::SetState(SetStateCommandValue::Off), }), - other => Err(FromPublishError::InvalidSetStateCommandPayload), + _other => Err(FromPublishError::InvalidSetStateCommandPayload), }, topic::fan_controller::fan_1::percentage::COMMAND => { - let payload = core::str::from_utf8(publish.payload) - .map_err(FromPublishError::InvalidStringPayload)?; + let payload = core::str::from_utf8(publish.payload)?; let set_point: SetPoint = payload.parse().map_err(FromPublishError::ParseSetPoint)?; @@ -425,11 +430,10 @@ impl TryFrom> for IncomingPublish { target: Fan::Two, command: FanCommand::SetState(SetStateCommandValue::Off), }), - other => Err(FromPublishError::InvalidSetStateCommandPayload), + _other => Err(FromPublishError::InvalidSetStateCommandPayload), }, topic::fan_controller::fan_2::percentage::COMMAND => { - let payload = core::str::from_utf8(publish.payload) - .map_err(FromPublishError::InvalidStringPayload)?; + let payload = core::str::from_utf8(publish.payload)?; let set_point: SetPoint = payload.parse().map_err(FromPublishError::ParseSetPoint)?; @@ -564,10 +568,10 @@ async fn mqtt_brain_routine( let publish = match message { Err(error) => { match error { - FromPublishError::InvalidStringPayload(utf8_error) => { + FromPublishError::InvalidStringPayload => { error!("Invalid UTF-8 payload"); } - FromPublishError::ParseSetPoint(parse_set_point_error) => { + FromPublishError::ParseSetPoint(_parse_set_point_error) => { error!("Invalid set point payload"); } FromPublishError::UnknownTopic => error!("Unknown topic. Look for ealier logs"), @@ -635,7 +639,7 @@ async fn fan_control_routine( let fan_identifier = match *fan_address { 2 => "[Fan 1]", 3 => "[Fan 2]", - other => "Unknown (oops)", + _other => "Unknown (oops)", }; info!("{} Waiting for MODBUS initialization", fan_identifier); @@ -674,7 +678,7 @@ async fn fan_control_routine( info!("{} Sending fan state update through modbus", fan_identifier); const MAX_ATTEMPTS: u8 = 3; let mut attempt = 1; - while let Err(error) = modbus.send_3(&function).await + while let Err(_error) = modbus.send_3(&function).await && attempt <= MAX_ATTEMPTS { // Release lock so other tasks get a chance to access modbus for sending messages to devices @@ -874,8 +878,6 @@ async fn main(spawner: Spawner) { PIN_25: pin_25, PIO0: pio0, DMA_CH0: dma_ch0, - DMA_CH1: dma_ch1, - DMA_CH2: dma_ch2, PIN_24: pin_24, PIN_29: pin_29, // Driver enable/disable pin to switch between sending and receiving data on UART/Modbus @@ -907,8 +909,6 @@ async fn main(spawner: Spawner) { pin_12, pin_13, Irqs, - dma_ch1, - dma_ch2, pin_4, tx_buffer, rx_buffer, diff --git a/fan-controller/src/modbus/client.rs b/fan-controller/src/modbus/client.rs index a592247..cc23646 100644 --- a/fan-controller/src/modbus/client.rs +++ b/fan-controller/src/modbus/client.rs @@ -1,35 +1,29 @@ -use core::ops::Deref; - use defmt::{error, info}; use embassy_rp::{ - Peripheral, dma, + Peripheral, gpio::{Level, Output, Pin}, interrupt::typelevel::Binding, uart::{self, BufferedInterruptHandler, BufferedUart, RxPin, TxPin}, }; -use embassy_time::{Duration, TimeoutError, Timer, block_for, with_timeout}; +use embassy_time::{Duration, TimeoutError, block_for, with_timeout}; use embedded_io_async::{Read, Write}; -use crate::{ - configuration, - fan::{BAUD_RATE, FanResponse, address, holding_registers, set_point::SetPoint}, - modbus::{self, function::WriteHoldingRegister}, -}; +use crate::{configuration, modbus::function::WriteHoldingRegister}; pub(crate) enum Error { Timeout(TimeoutError), - Uart(uart::Error), + Uart, } -impl From for Error { - fn from(error: TimeoutError) -> Self { - Self::Timeout(error) +impl From for Error { + fn from(_value: uart::Error) -> Self { + Self::Uart } } -impl From for Error { - fn from(error: uart::Error) -> Self { - Self::Uart(error) +impl From for Error { + fn from(error: TimeoutError) -> Self { + Self::Timeout(error) } } @@ -48,8 +42,6 @@ impl<'a, UART: uart::Instance, PIN: Pin> Client<'a, UART, PIN> { tx: impl Peripheral

> + 'a, rx: impl Peripheral

> + 'a, irq: impl Binding>, - tx_dma: impl Peripheral

+ 'a, - rx_dma: impl Peripheral

+ 'a, driver_enable: impl Peripheral

+ 'a, tx_buffer: &'a mut [u8], rx_buffer: &'a mut [u8], @@ -64,61 +56,6 @@ impl<'a, UART: uart::Instance, PIN: Pin> Client<'a, UART, PIN> { } } - pub(crate) async fn send_2( - &mut self, - message: impl modbus::ToBytes, - ) -> Result, Error> { - // Write then read - // Set pin setting DE (driver enable) to on (high) on the MAX845 to send data - self.driver_enable.set_high(); - - let bytes = message.to_bytes(); - info!("Sending message to fan: {:?}", bytes); - // As ref because &[u8; 8] is not the same as &[u8] - let result = with_timeout(configuration::FAN_TIMEOUT, self.uart.write_all(&bytes)).await?; - - info!("uart write result: {:?}", result); - - // Before closing we need to flush the buffer to ensure that all data is written - // This requires blocking or we get a WouldBlock error. I don't understand why (TODO) - let result = self.uart.blocking_flush(); - if let Err(_) = result { - error!("uart flush error"); - } - - // In addition to flushing we need to wait for some time before turning off data in on the - // MAX845 because we might be too fast and cut off the last byte or more. (This happened) - // I saw someone using 120 microseconds (https://youtu.be/i46jdhvRej4?t=886). - // This number is based on trial and error. Don't feel bad to change it if it doesn't work. - // Also timings in microseconds are not accurate. - // I assume this should be below the modbus message delay - // Timer::after(Duration::from_micros(1_000)).await; - // Using await timer breaks this too. Probably because it yields to the scheduler - block_for(BLOCK_FOR); - - // Close sending data to enable receiving data - self.driver_enable.set_low(); - - // Read - // Read response from fan. The response can vary in length - let mut response_buffer: [u8; RESPONSE] = [0; RESPONSE]; - info!("Waiting for response from fan"); - let bytes_read = with_timeout( - configuration::FAN_TIMEOUT, - //TODO test this does not wait for bytes to fill the buffer - // leading to a timeout because the response is only 7 bytes but the buffer is 8 and it waits for the last byte to arrive - self.uart.read(&mut response_buffer), - ) - .await??; - - info!("response from fan: {:?} {:?}", bytes_read, response_buffer); - let response = FanResponse::new(response_buffer, bytes_read); - - //TODO validate response from fan - // Read the correct number of bytes - Ok(response) - } - pub(crate) async fn send_3(&mut self, message: &WriteHoldingRegister) -> Result<(), Error> { // For debugging let fan_identifier = match *message.device_address() { @@ -179,114 +116,4 @@ impl<'a, UART: uart::Instance, PIN: Pin> Client<'a, UART, PIN> { // Read the correct number of bytes Ok(()) } - - async fn send( - &mut self, - message: impl AsRef<[u8]>, - ) -> Result, Error> { - // Write then read - // Set pin setting DE (driver enable) to on (high) on the MAX845 to send data - self.driver_enable.set_high(); - - info!("Sending message to fan: {:?}", message.as_ref()); - // As ref because &[u8; 8] is not the same as &[u8] - let result = with_timeout( - configuration::FAN_TIMEOUT, - self.uart.write_all(message.as_ref()), - ) - .await?; - - info!("uart write result: {:?}", result); - - // Before closing we need to flush the buffer to ensure that all data is written - // This requires blocking or we get a WouldBlock error. I don't understand why (TODO) - let result = self.uart.blocking_flush(); - if let Err(_error) = result { - error!("uart flush error"); - } - - // In addition to flushing we need to wait for some time before turning off data in on the - // MAX845 because we might be too fast and cut off the last byte or more. (This happened) - // I saw someone using 120 microseconds (https://youtu.be/i46jdhvRej4?t=886). - // This number is based on trial and error. Don't feel bad to change it if it doesn't work. - // Also timings in microseconds are not accurate. - // I assume this should be below the modbus message delay - // Timer::after(Duration::from_micros(1_000)).await; - // Using await timer breaks this too. Probably because it yields to the scheduler - block_for(BLOCK_FOR); - - // Close sending data to enable receiving data - self.driver_enable.set_low(); - - // Read - // Read response from fan. The response can vary in length - let mut response_buffer: [u8; N] = [0; N]; - info!("Waiting for response from fan"); - let bytes_read = with_timeout( - configuration::FAN_TIMEOUT, - //TODO test this does not wait for bytes to fill the buffer - // leading to a timeout because the response is only 7 bytes but the buffer is 8 and it waits for the last byte to arrive - self.uart.read(&mut response_buffer), - ) - .await??; - - info!("response from fan: {:?} {:?}", bytes_read, response_buffer); - let response = FanResponse::new(response_buffer, bytes_read); - - //TODO validate response from fan - // Read the correct number of bytes - Ok(response) - } - - //TODO decouple - /// The mutable reference to self here is important as there can only be one writer to the (mod)bus at a time - #[deprecated(note = "Decoupled fan from modbus")] - pub(crate) async fn set_set_point(&mut self, set_point: &SetPoint) -> Result<(), Error> { - // Send update through UART to MAX845 to modbus fans - // Form message to fan 1 - let register_address = (*holding_registers::REFERENCE_SET_POINT).to_be_bytes(); - let value: u16 = *set_point.deref(); - let mut message: [u8; 8] = [ - // Device address fan 1 - *address::FAN_1, - // Modbus function code - modbus::function::code::WRITE_SINGLE_REGISTER, - // Holding register address - register_address[0], - register_address[1], - // Value to set - (value >> 8) as u8, - value as u8, - // CRC is set later - 0, - 0, - ]; - - let checksum = modbus::CRC.checksum(&message[..6]).to_be_bytes(); - - // They come out reversed (or is us using to_be_bytes reversed?) - message[6] = checksum[1]; - message[7] = checksum[0]; - info!("Sending message to fan 1: {:?}", message); - - let _ = self.send::<8>(&message).await?; - - /// Messsage delay between modbus messages in microseconds - const MESSAGE_DELAY: u64 = modbus::get_message_delay(BAUD_RATE); - info!("Message delay {}", MESSAGE_DELAY); - // We can yield the future here because the wait time between messages is a minimum and can be longer - Timer::after_micros(MESSAGE_DELAY).await; - - // Form message to fan 2 - // Update the fan address and therefore the CRC - // Keep speed as both fans should be running at the same speed - message[0] = *address::FAN_2; - let checksum = modbus::CRC.checksum(&message[..6]).to_be_bytes(); - message[6] = checksum[1]; - message[7] = checksum[0]; - - info!("sending message to fan 2: {:?}", message); - let _ = self.send::<8>(&message).await?; - Ok(()) - } } diff --git a/fan-controller/src/modbus/function/code.rs b/fan-controller/src/modbus/function/code.rs index 5fa6527..d1becfb 100644 --- a/fan-controller/src/modbus/function/code.rs +++ b/fan-controller/src/modbus/function/code.rs @@ -1,3 +1 @@ -pub const READ_HOLDING_REGISTER: u8 = 0x03; -pub const READ_INPUT_REGISTER: u8 = 0x04; pub const WRITE_SINGLE_REGISTER: u8 = 0x06; diff --git a/fan-controller/src/modbus/function/mod.rs b/fan-controller/src/modbus/function/mod.rs index 11bc9d8..ac46344 100644 --- a/fan-controller/src/modbus/function/mod.rs +++ b/fan-controller/src/modbus/function/mod.rs @@ -1,8 +1,4 @@ pub(super) mod code; -pub(crate) mod read_input_register; pub(crate) mod write_holding_register; pub(crate) use write_holding_register::WriteHoldingRegister; -pub(crate) trait Function { - const CODE: u8; -} diff --git a/fan-controller/src/modbus/function/read_input_register.rs b/fan-controller/src/modbus/function/read_input_register.rs deleted file mode 100644 index 3d50ef5..0000000 --- a/fan-controller/src/modbus/function/read_input_register.rs +++ /dev/null @@ -1,19 +0,0 @@ -use crate::modbus::function::{self, Function}; - -pub(crate) struct ReadInputRegister { - pub(crate) address: u16, - pub(crate) number_of_registers: u16, -} - -impl ReadInputRegister { - pub(crate) fn new(address: u16, number_of_registers: u16) -> Self { - Self { - address, - number_of_registers, - } - } -} - -impl Function for ReadInputRegister { - const CODE: u8 = function::code::READ_INPUT_REGISTER; -} diff --git a/fan-controller/src/modbus/mod.rs b/fan-controller/src/modbus/mod.rs index 92b6d5a..c295896 100644 --- a/fan-controller/src/modbus/mod.rs +++ b/fan-controller/src/modbus/mod.rs @@ -6,79 +6,6 @@ pub(crate) mod register; use crc::{CRC_16_MODBUS, Crc}; pub(crate) use client::Client; -use function::{Function, read_input_register::ReadInputRegister}; /// Used to create CRC checksums when forming modbus messages pub(super) const CRC: Crc = Crc::::new(&CRC_16_MODBUS); - -/// Calculate the time in microseconds that needs to be waited between sending modbus messages -/// Modbus delay between messages is required to be 3.5 character times (bytes). -/// The function will return more than the required time if the calculation (division) does not -/// result in a whole number as it is better to wait longer than too short. -pub(super) const fn get_message_delay(baud_rate: u32) -> u64 { - /// Modbus delay between messages in bits - /// The modbus delay between messages is 3.5 bytes or 28 bits - const DELAY_BITS: u64 = 28 + 4; - - // To send one bit it takes 1/19200 seconds. - // To send one bit it takes 1 * 1,000,000 / 19200 microseconds. - // To send 3.5 bytes it takes 1,000,000 * 28 / 19200 microseconds. - // Putting the division last to avoid add on effect of inaccurate floating point division - const MICROSECONDS_FOR_BITS: u64 = 1_000_000 * DELAY_BITS; - - // Using floating point division to be closer to the logical mathematical result - // (e.g. 3 / 2 = 1.5 instead of 1 with integer division) - - // Round up as it is better to wait longer than too short - MICROSECONDS_FOR_BITS.div_ceil(baud_rate as u64) -} - -pub(crate) struct Message { - device_address: u8, - pub(crate) function: F, -} - -pub(crate) trait ToBytes { - fn to_bytes(&self) -> [u8; LENGTH]; -} - -impl ToBytes<8> for Message { - fn to_bytes(&self) -> [u8; 8] { - let address_bytes = self.function.address.to_be_bytes(); - let length_bytes = self.function.number_of_registers.to_be_bytes(); - let mut buffer = [ - // Device address - self.device_address, - // Modbus function code - self.code(), - // Starting address - address_bytes[0], - address_bytes[1], - // Number of registers to read - length_bytes[0], - length_bytes[1], - // CRC checksum (placeholder) - 0, - 0, - ]; - - let checksum = CRC.checksum(&buffer[..6]).to_be_bytes(); - // They come out reversed (or is us using to_be_bytes reversed?) - buffer[6] = checksum[1]; - buffer[7] = checksum[0]; - buffer - } -} - -impl Message { - pub(crate) fn new(address: u8, function: F) -> Self { - Self { - device_address: address, - function, - } - } - - const fn code(&self) -> u8 { - F::CODE - } -} diff --git a/fan-controller/src/mqtt/mod.rs b/fan-controller/src/mqtt/mod.rs index 2e60421..b430ac6 100644 --- a/fan-controller/src/mqtt/mod.rs +++ b/fan-controller/src/mqtt/mod.rs @@ -7,7 +7,6 @@ use defmt::Format; pub(crate) mod packet; pub(crate) mod task; -pub(crate) mod v2; pub(crate) mod variable_byte_integer; #[derive(Debug, Format, Clone)] diff --git a/fan-controller/src/mqtt/packet/connect.rs b/fan-controller/src/mqtt/packet/connect.rs index 5c76e99..b8160fa 100644 --- a/fan-controller/src/mqtt/packet/connect.rs +++ b/fan-controller/src/mqtt/packet/connect.rs @@ -15,102 +15,6 @@ pub(crate) struct Connect<'a> { impl<'a> Connect<'a> { pub(crate) const TYPE: u8 = 1; - - #[deprecated(note = "Use Encode trait")] - pub(crate) fn encode(&self, buffer: &mut [u8], offset: &mut usize) -> Result<(), EncodeError> { - let remaining_length = 11 - + size_of::() - + self.client_identifier.len() - + size_of::() - + self.username.len() - + size_of::() - + self.password.len(); - - let required_length = size_of_val(&Self::TYPE) + remaining_length; - if required_length > buffer.len() - *offset { - return Err(EncodeError::BufferTooSmall { - required: required_length, - available: buffer.len() - *offset, - }); - } - - // Fixed header - buffer[*offset] = Self::TYPE << 4; - *offset += 1; - - variable_byte_integer::encode(remaining_length, buffer, offset) - .map_err(EncodeError::WriteRemainingLengthError)?; - - // Variable header - // Protocol name length - buffer[*offset] = 0x00; - *offset += 1; - buffer[*offset] = 4; - *offset += 1; - - // Protocol name - buffer[*offset] = b'M'; - *offset += 1; - buffer[*offset] = b'Q'; - *offset += 1; - buffer[*offset] = b'T'; - *offset += 1; - buffer[*offset] = b'T'; - *offset += 1; - // Protocol version - buffer[*offset] = 5; - *offset += 1; - - // Connect Flags - // USER_NAME_FLAG | PASSWORD_FLAG | CLEAN_START - buffer[*offset] = 0b1100_0010; - *offset += 1; - - // Keep alive - buffer[*offset] = (self.keep_alive_seconds >> 8) as u8; - *offset += 1; - buffer[*offset] = self.keep_alive_seconds as u8; - *offset += 1; - // Property length 0 (no properties). Has to be set to 0 if there are no properties - buffer[*offset] = 0; - *offset += 1; - - // Payload - // Client identifier - let length = self.client_identifier.len(); - buffer[*offset] = (length >> 8) as u8; - *offset += 1; - buffer[*offset] = length as u8; - *offset += 1; - - for byte in self.client_identifier.as_bytes() { - buffer[*offset] = *byte; - *offset += 1; - } - // Username - let length = self.username.len(); - buffer[*offset] = (length >> 8) as u8; - *offset += 1; - buffer[*offset] = length as u8; - *offset += 1; - for byte in self.username.as_bytes() { - buffer[*offset] = *byte; - *offset += 1; - } - - // Password - let length = self.password.len(); - buffer[*offset] = (length >> 8) as u8; - *offset += 1; - buffer[*offset] = length as u8; - *offset += 1; - for byte in self.password { - buffer[*offset] = *byte; - *offset += 1; - } - - Ok(()) - } } impl TryEncode for Connect<'_> { @@ -234,8 +138,6 @@ impl<'a> TryFrom<&MqttBrokerConfiguration<'a>> for Connect<'a> { #[derive(Debug, Format)] pub(crate) enum EncodeError { EmptyBuffer, - /// Client identifier + user name + password together are larger than [VariableByteInteger::MAX] - DataTooLarge, /// The buffer does not contain enough empty space to write the packet BufferTooSmall { required: usize, diff --git a/fan-controller/src/mqtt/packet/connect_acknowledgement.rs b/fan-controller/src/mqtt/packet/connect_acknowledgement.rs index f92584d..732a98c 100644 --- a/fan-controller/src/mqtt/packet/connect_acknowledgement.rs +++ b/fan-controller/src/mqtt/packet/connect_acknowledgement.rs @@ -40,7 +40,7 @@ impl TryDecode<'_> for ConnectAcknowledgement { type Error = DecodeError; /// Reads the variable header and payload of a connect acknowledgement packet - fn try_decode(flags: u8, buffer: &[u8]) -> Result + fn try_decode(_flags: u8, buffer: &[u8]) -> Result where Self: Sized, { diff --git a/fan-controller/src/mqtt/packet/disconnect.rs b/fan-controller/src/mqtt/packet/disconnect.rs index 1d92328..6ef5696 100644 --- a/fan-controller/src/mqtt/packet/disconnect.rs +++ b/fan-controller/src/mqtt/packet/disconnect.rs @@ -93,7 +93,7 @@ pub(crate) enum DecodeDisconnectError { impl TryDecode<'_> for Disconnect { type Error = DecodeDisconnectError; - fn try_decode(flags: u8, variable_header_and_payload: &[u8]) -> Result { + fn try_decode(_flags: u8, variable_header_and_payload: &[u8]) -> Result { // Variable header // Disconnect reason code let reason_code = variable_header_and_payload[0]; diff --git a/fan-controller/src/mqtt/packet/mod.rs b/fan-controller/src/mqtt/packet/mod.rs index 54872a7..61e85db 100644 --- a/fan-controller/src/mqtt/packet/mod.rs +++ b/fan-controller/src/mqtt/packet/mod.rs @@ -1,15 +1,6 @@ -use crate::mqtt::packet::connect::Connect; -use crate::mqtt::packet::connect_acknowledgement::ConnectAcknowledgement; -use crate::mqtt::packet::publish::Publish; -use crate::mqtt::packet::subscribe::Subscribe; -use crate::mqtt::packet::subscribe_acknowledgement::{ - SubscribeAcknowledgement, SubscribeAcknowledgementError, -}; use crate::mqtt::variable_byte_integer; use defmt::Format; -use super::TryDecode; - pub(crate) mod connect; pub(crate) mod connect_acknowledgement; pub(crate) mod disconnect; @@ -25,41 +16,13 @@ pub(crate) enum GetPartsError { InvalidRemainingLength(variable_byte_integer::DecodeError), MissingBytes(usize), } - -#[derive(Debug, Clone, Format)] -pub(crate) enum ReadError { - /// The packet type is not supported. This can happen if there is a packet received that is - /// only intended for the broker and not the client. Or the packet type is not yet implemented. - UnsupportedPacketType(u8), - UnexpectedPacketType(u8), - PartsError(GetPartsError), - ConnectAcknowledgementError(connect_acknowledgement::DecodeError), - PublishError(publish::ReadError), - SubscribeAcknowledgementError(SubscribeAcknowledgementError), -} - pub(crate) struct PacketParts<'a> { pub(crate) r#type: u8, pub(crate) flags: u8, pub(crate) variable_header_and_payload: &'a [u8], } -/// T is for users of this MQTT implementation to define as the publish packets they expect vary by -/// application. The only requirement is that they can be created from a publish packet which -/// contains the topic name and payload. This is to get around the problem that publish topic names -/// and payloads can have a variable unknown length and are difficult to pass around with lifetimes. -#[derive(Format)] -pub(crate) enum Packet -where - T: FromPublish, - S: FromSubscribeAcknowledgement, -{ - ConnectAcknowledgement(ConnectAcknowledgement), - SubscribeAcknowledgement(S), - Publish(T), -} - -pub(crate) fn get_parts(buffer: &[u8]) -> Result { +pub(crate) fn get_parts(buffer: &'_ [u8]) -> Result, GetPartsError> { if buffer.is_empty() { return Err(GetPartsError::EmptyBuffer); } @@ -83,68 +46,3 @@ pub(crate) fn get_parts(buffer: &[u8]) -> Result { variable_header_and_payload, }) } - -impl Packet -where - T: FromPublish, - S: FromSubscribeAcknowledgement, -{ - pub(crate) fn read(buffer: &[u8]) -> Result, ReadError> { - // Fixed header - - let parts = get_parts(buffer).map_err(ReadError::PartsError)?; - - match parts.r#type { - Connect::TYPE => Err(ReadError::UnsupportedPacketType(parts.r#type)), - ConnectAcknowledgement::TYPE => { - let connect_acknowledgement = ConnectAcknowledgement::try_decode( - parts.flags, - parts.variable_header_and_payload, - ) - .map_err(ReadError::ConnectAcknowledgementError)?; - - Ok(Packet::ConnectAcknowledgement(connect_acknowledgement)) - } - Publish::TYPE => { - let publish = Publish::try_decode(parts.flags, parts.variable_header_and_payload) - .map_err(ReadError::PublishError)?; - - let packet = T::from_publish(publish); - Ok(Packet::Publish(packet)) - } - Subscribe::TYPE => Err(ReadError::UnsupportedPacketType(parts.r#type)), - SubscribeAcknowledgement::TYPE => { - let subscribe_acknowledgement = - SubscribeAcknowledgement::read(parts.variable_header_and_payload) - .map_err(ReadError::SubscribeAcknowledgementError)?; - - Ok(Packet::SubscribeAcknowledgement( - S::from_subscribe_acknowledgement(subscribe_acknowledgement), - )) - } - - unexpected => Err(ReadError::UnexpectedPacketType(unexpected)), - } - } - - pub(crate) const fn get_type(&self) -> u8 { - match self { - Packet::ConnectAcknowledgement(_) => ConnectAcknowledgement::TYPE, - Packet::SubscribeAcknowledgement(_) => SubscribeAcknowledgement::TYPE, - Packet::Publish(_) => Publish::TYPE, - } - } -} - -pub(crate) trait FromPublish { - fn from_publish(publish: Publish) -> Self; -} - -/// Temporary just for testing. TODO remove -impl FromPublish for () { - fn from_publish(publish: Publish) -> Self {} -} - -pub(crate) trait FromSubscribeAcknowledgement { - fn from_subscribe_acknowledgement(subscribe_acknowledgement: SubscribeAcknowledgement) -> Self; -} diff --git a/fan-controller/src/mqtt/packet/publish.rs b/fan-controller/src/mqtt/packet/publish.rs index 920353e..a7ca85e 100644 --- a/fan-controller/src/mqtt/packet/publish.rs +++ b/fan-controller/src/mqtt/packet/publish.rs @@ -165,9 +165,3 @@ impl<'a> TryDecode<'a> for Publish<'a> { }) } } - -mod v2 { - pub(crate) struct Publish { - buffer: [u8; 1024], - } -} diff --git a/fan-controller/src/mqtt/packet/subscribe.rs b/fan-controller/src/mqtt/packet/subscribe.rs index dd70552..824956b 100644 --- a/fan-controller/src/mqtt/packet/subscribe.rs +++ b/fan-controller/src/mqtt/packet/subscribe.rs @@ -14,18 +14,12 @@ pub(crate) struct Options(u8); pub(crate) enum RetainHandling { // Send retained messages at the time of subscribe (0) SendAtSubscribe, - // Send retained messages only if the subscription does not currently exist (1) - OnlyIfNotExists, - // Do not send retained messages (2) - DoNotSend, } impl RetainHandling { const fn to_byte(&self) -> u8 { match self { RetainHandling::SendAtSubscribe => 0, - RetainHandling::OnlyIfNotExists => 1, - RetainHandling::DoNotSend => 2, } } } diff --git a/fan-controller/src/mqtt/packet/subscribe_acknowledgement.rs b/fan-controller/src/mqtt/packet/subscribe_acknowledgement.rs index 38aa66c..a3fb260 100644 --- a/fan-controller/src/mqtt/packet/subscribe_acknowledgement.rs +++ b/fan-controller/src/mqtt/packet/subscribe_acknowledgement.rs @@ -1,5 +1,4 @@ use crate::mqtt::variable_byte_integer; -use ::mqtt::QualityOfService; use defmt::Format; #[derive(Debug, Clone, Format)] @@ -7,25 +6,6 @@ pub(crate) enum SubscribeAcknowledgementError { InvalidPropertiesLength(variable_byte_integer::DecodeError), } -#[derive(Format)] -pub(crate) enum SubscribeErrorReasonCode { - UnspecifiedError = 0x80, - ImplementationSpecificError = 0x83, - NotAuthorized = 0x87, - TopicFilterInvalid = 0x8F, - PacketIdentifierInUse = 0x91, - QuotaExceeded = 0x97, - SharedSubscriptionsNotSupported = 0x9E, - SubscriptionIdentifiersNotSupported = 0xA1, - WildcardSubscriptionsNotSupported = 0xA2, -} - -#[derive(Format)] -pub(crate) enum SubscribeReasonCode { - GrantedQualityOfService(QualityOfService), - ErrorCode(SubscribeErrorReasonCode), -} - #[derive(Format)] pub(crate) struct SubscribeAcknowledgement<'a> { pub(crate) packet_identifier: u16, diff --git a/fan-controller/src/mqtt/v2/mod.rs b/fan-controller/src/mqtt/v2/mod.rs deleted file mode 100644 index ecd84f4..0000000 --- a/fan-controller/src/mqtt/v2/mod.rs +++ /dev/null @@ -1,2 +0,0 @@ -mod packet; -mod publish; diff --git a/fan-controller/src/mqtt/v2/packet.rs b/fan-controller/src/mqtt/v2/packet.rs deleted file mode 100644 index 87eb13a..0000000 --- a/fan-controller/src/mqtt/v2/packet.rs +++ /dev/null @@ -1,64 +0,0 @@ -use core::ops::Range; - -use defmt::Format; - -use crate::mqtt::variable_byte_integer; - -#[derive(Clone, Format, Debug)] -pub(crate) enum PacketError { - EmptyBuffer, - InvalidRemainingLength(variable_byte_integer::DecodeError), - MissingBytes(usize), -} - -trait Packetter { - fn test() { - todo!() - } -} - -enum Packett {} - -struct Packet { - buffer: [u8; L], - variable_header_and_payload: Range, -} - -impl Packet { - fn r#type(&self) -> u8 { - self.buffer[0] >> 4 - } - - fn flags(&self) -> u8 { - self.buffer[0] & 0b0000_1111 - } - - fn decode(self) -> Result { - todo!() - } -} - -impl TryFrom<[u8; L]> for Packet { - type Error = PacketError; - - fn try_from(buffer: [u8; L]) -> Result { - if buffer.is_empty() { - return Err(PacketError::EmptyBuffer); - } - - // Skip fixed header - let mut offset = 1; - let remaining_length = variable_byte_integer::decode(&buffer, &mut offset) - .map_err(PacketError::InvalidRemainingLength)?; - - if buffer.len() < offset + remaining_length { - let missing_bytes = buffer.len() - offset - remaining_length; - return Err(PacketError::MissingBytes(missing_bytes)); - } - - Ok(Self { - buffer, - variable_header_and_payload: offset..offset + remaining_length, - }) - } -} diff --git a/fan-controller/src/mqtt/v2/publish.rs b/fan-controller/src/mqtt/v2/publish.rs deleted file mode 100644 index c18ba85..0000000 --- a/fan-controller/src/mqtt/v2/publish.rs +++ /dev/null @@ -1 +0,0 @@ -struct Publish {} diff --git a/fan-controller/src/mqtt/variable_byte_integer.rs b/fan-controller/src/mqtt/variable_byte_integer.rs index 06bdba0..4f030b8 100644 --- a/fan-controller/src/mqtt/variable_byte_integer.rs +++ b/fan-controller/src/mqtt/variable_byte_integer.rs @@ -1,12 +1,13 @@ use defmt::Format; #[derive(Debug, Format)] -pub(super) enum VariableByteIntegerEncodeError { +pub(crate) enum VariableByteIntegerEncodeError { /// The integer is larger than 268,435,455 ([VariableByteInteger::MAX]). TooLarge, /// The buffer is too small to write the integer EndOfBuffer, } + const MAX: usize = 268_435_455; /// Returns bytes written on success pub(super) fn encode( @@ -52,7 +53,7 @@ pub(super) fn encode( } #[derive(Debug, Format, Clone)] -pub(super) enum DecodeError { +pub(crate) enum DecodeError { MalformedVariableByteIntegerError, /// The buffer does not contain enough bytes to read the remaining length EndOfBuffer, diff --git a/fan-controller/src/task.rs b/fan-controller/src/task.rs index 674bfb5..9e8a06f 100644 --- a/fan-controller/src/task.rs +++ b/fan-controller/src/task.rs @@ -1,5 +1,4 @@ use crate::PingRequest; -use crate::fan::set_point::SetPoint; use crate::mqtt::packet::connect::Connect; use crate::mqtt::packet::disconnect::Disconnect; use crate::mqtt::packet::ping_response::PingResponse; @@ -9,7 +8,7 @@ use crate::mqtt::packet::subscribe_acknowledgement::SubscribeAcknowledgement; use crate::mqtt::task::send; use crate::mqtt::{self}; use crate::mqtt::{TryDecode, non_zero_u16}; -use crate::{configuration, fan, gain_control}; +use crate::{configuration, gain_control}; use ::mqtt::QualityOfService; use core::future::poll_fn; use core::num::NonZeroU16; @@ -50,7 +49,7 @@ async fn handle_subscribe_acknowledgement<'f, const SUBSCRIPTIONS: usize>( let mut acknowledgements = acknowledgements.lock().await; info!("[Subscription] Locked ACKNOWLEDGEMENTS"); // Validate server sends a valid packet identifier or we get bamboozled and panic - let Some(value) = acknowledgements.get_mut(acknowledgement.packet_identifier as usize) else { + let Some(_value) = acknowledgements.get_mut(acknowledgement.packet_identifier as usize) else { warn!( "[Subscription] Received subscribe acknowledgement for out of bounds packet identifier" ); @@ -103,8 +102,6 @@ async fn handle_ping_response( } enum ClientState { - Disconnected, - Connected, ConnectionLost, } @@ -227,22 +224,6 @@ async fn listen< } } -enum PredefinedPublish { - FanPercentageState { - setting: SetPoint, - }, - FanOnState { - is_on: bool, - }, - SensorTemperature { - /// Celsius temperature as read from the sensor. This is the raw value. To get the actual temperature, divide by 10. - /// e.g. 234 means 23.4 degrees Celsius - temperature: u16, - /// The fan the sensor is on - fan: fan::Fan, - }, -} - enum Message<'a, T: Publish> { Subscribe(Subscribe<'a>), Publish(T),