From a72365043dc20552a5cee669ebedf8e852926958 Mon Sep 17 00:00:00 2001 From: Sebastian Scholz Date: Fri, 3 Oct 2025 20:56:20 +0200 Subject: [PATCH 1/3] Allow to construct a coil from an iterator --- src/codec/mod.rs | 16 +++-- src/frame/coils.rs | 166 +++++++++++++++++++++++++++++++++------------ src/frame/mod.rs | 6 +- 3 files changed, 136 insertions(+), 52 deletions(-) diff --git a/src/codec/mod.rs b/src/codec/mod.rs index db5bb9f..8bf26fa 100644 --- a/src/codec/mod.rs +++ b/src/codec/mod.rs @@ -110,7 +110,9 @@ impl<'r> TryFrom<&'r [u8]> for Request<'r> { ), F::WriteMultipleCoils => { let address = BigEndian::read_u16(&bytes[1..3]); - let quantity = BigEndian::read_u16(&bytes[3..5]) as usize; + let quantity = CoilQuantity { + quantity: BigEndian::read_u16(&bytes[3..5]) as usize, + }; let byte_count = bytes[5]; if bytes.len() < (6 + byte_count as usize) { return Err(Error::ByteCount(byte_count)); @@ -179,7 +181,9 @@ impl<'r> TryFrom<&'r [u8]> for Response<'r> { let data = &bytes[2..byte_count + 2]; // Here we have not information about the exact requested quantity // therefore we just assume that the whole byte is meant. - let quantity = byte_count * 8; + let quantity = CoilQuantity { + quantity: byte_count * 8, + }; match FunctionCode::new(fn_code) { FunctionCode::ReadCoils => Self::ReadCoils(Coils { data, quantity }), @@ -689,7 +693,7 @@ mod tests { Request::WriteMultipleCoils( 0x3311, Coils { - quantity: 4, + quantity: CoilQuantity { quantity: 4 }, data: &[0b1101] } ) @@ -936,7 +940,7 @@ mod tests { assert_eq!( rsp, Response::ReadCoils(Coils { - quantity: 8, + quantity: CoilQuantity { quantity: 8 }, data: &[0b_0000_1001] }) ); @@ -949,7 +953,7 @@ mod tests { assert_eq!( rsp, Response::ReadCoils(Coils { - quantity: 0, + quantity: CoilQuantity { quantity: 0 }, data: &[] }) ); @@ -968,7 +972,7 @@ mod tests { assert_eq!( rsp, Response::ReadDiscreteInputs(Coils { - quantity: 8, + quantity: CoilQuantity { quantity: 8 }, data: &[0b_0000_1001] }) ); diff --git a/src/frame/coils.rs b/src/frame/coils.rs index d57429e..7742fb2 100644 --- a/src/frame/coils.rs +++ b/src/frame/coils.rs @@ -9,19 +9,24 @@ use crate::error::*; #[derive(Debug, Clone, Copy, PartialEq, Eq)] pub struct Coils<'c> { pub(crate) data: RawData<'c>, - pub(crate) quantity: usize, + pub(crate) quantity: CoilQuantity, } impl<'c> Coils<'c> { /// Pack coils defined by an bool slice into a byte buffer. pub fn from_bools(bools: &[bool], target: &'c mut [u8]) -> Result { - if bools.is_empty() { - return Err(Error::BufferSize); - } - pack_coils(bools, target)?; + Self::from_iter(bools.iter().copied(), target) + } + + /// Pack coils from an iterator into a byte buffer. + pub fn from_iter( + bools: impl IntoIterator, + target: &'c mut [u8], + ) -> Result { + let quantity = pack_coils(bools, target)?; Ok(Coils { data: target, - quantity: bools.len(), + quantity, }) } @@ -37,25 +42,25 @@ impl<'c> Coils<'c> { /// Quantity of coils #[must_use] pub const fn len(&self) -> usize { - self.quantity + self.quantity.quantity } /// Number of bytes required to pack the coils. #[must_use] pub const fn packed_len(&self) -> usize { - packed_coils_len(self.quantity) + self.quantity.packed_len() } /// Returns `true` if the container has no items. #[must_use] pub const fn is_empty(&self) -> bool { - self.quantity == 0 + self.quantity.quantity == 0 } /// Get a specific coil. #[must_use] pub const fn get(&self, idx: usize) -> Option { - if idx + 1 > self.quantity { + if idx + 1 > self.quantity.quantity { return None; } Some((self.data[(idx as u16 / 8u16) as usize] >> (idx % 8)) & 0b1 > 0) @@ -108,25 +113,43 @@ pub const fn u16_coil_to_bool(coil: u16) -> Result { } } -/// Calculate the number of bytes required for a given number of coils. -#[must_use] -pub const fn packed_coils_len(bitcount: usize) -> usize { - bitcount.div_ceil(8) +/// A quantity of coils +#[cfg_attr(all(feature = "defmt", target_os = "none"), derive(defmt::Format))] +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub struct CoilQuantity { + pub(crate) quantity: usize, +} + +impl CoilQuantity { + /// Calculate the number of bytes required the number of coils. + #[must_use] + pub const fn packed_len(&self) -> usize { + self.quantity.div_ceil(8) + } } /// Pack coils into a byte array. /// -/// It returns the number of bytes used to pack the coils. -pub fn pack_coils(coils: &[Coil], bytes: &mut [u8]) -> Result { - let packed_size = packed_coils_len(coils.len()); - if bytes.len() < packed_size { - return Err(Error::BufferSize); +/// It returns the number of coils. +pub fn pack_coils( + coils: impl IntoIterator, + bytes: &mut [u8], +) -> Result { + let mut coil_count = 0; + for coil in coils { + let value = u8::from(coil); + let Some(byte) = bytes.get_mut(coil_count / 8) else { + return Err(Error::BufferSize); + }; + *byte |= value << (coil_count % 8); + match coil_count.checked_add(1) { + Some(count) => coil_count = count, + None => return Err(Error::ByteCount(0)), + } } - coils.iter().enumerate().for_each(|(i, b)| { - let v = u8::from(*b); - bytes[i / 8] |= v << (i % 8); - }); - Ok(packed_size) + Ok(CoilQuantity { + quantity: coil_count, + }) } /// Unpack coils from a byte array. @@ -159,11 +182,25 @@ mod tests { assert_eq!(iter.next(), None); } + #[test] + fn from_iterator() { + let iterator = [0, 1, 2, 3].iter().map(|value| value % 2 == 0); + let buff: &mut [u8] = &mut [0]; + let coils = Coils::from_iter(iterator, buff).unwrap(); + assert_eq!(coils.len(), 4); + let mut iter = coils.into_iter(); + assert_eq!(iter.next(), Some(true)); + assert_eq!(iter.next(), Some(false)); + assert_eq!(iter.next(), Some(true)); + assert_eq!(iter.next(), Some(false)); + assert_eq!(iter.next(), None); + } + #[test] fn coils_len() { let coils = Coils { data: &[0, 1, 2], - quantity: 5, + quantity: CoilQuantity { quantity: 5 }, }; assert_eq!(coils.len(), 5); } @@ -172,7 +209,7 @@ mod tests { fn coils_empty() { let coils = Coils { data: &[0, 1, 2], - quantity: 0, + quantity: CoilQuantity { quantity: 0 }, }; assert!(coils.is_empty()); } @@ -181,14 +218,14 @@ mod tests { fn coils_get() { let coils = Coils { data: &[0b1], - quantity: 1, + quantity: CoilQuantity { quantity: 1 }, }; assert_eq!(coils.get(0), Some(true)); assert_eq!(coils.get(1), None); let coils = Coils { data: &[0b01], - quantity: 2, + quantity: CoilQuantity { quantity: 2 }, }; assert_eq!(coils.get(0), Some(true)); assert_eq!(coils.get(1), Some(false)); @@ -196,7 +233,7 @@ mod tests { let coils = Coils { data: &[0xff, 0b11], - quantity: 10, + quantity: CoilQuantity { quantity: 10 }, }; for i in 0..10 { assert_eq!(coils.get(i), Some(true)); @@ -208,7 +245,7 @@ mod tests { fn coils_iter() { let coils = Coils { data: &[0b0101_0011], - quantity: 5, + quantity: CoilQuantity { quantity: 5 }, }; let mut coils_iter = CoilsIter { cnt: 0, coils }; assert_eq!(coils_iter.next(), Some(true)); @@ -223,7 +260,7 @@ mod tests { fn coils_into_iter() { let coils = Coils { data: &[0b0101_0011], - quantity: 3, + quantity: CoilQuantity { quantity: 3 }, }; let mut coils_iter = coils.into_iter(); assert_eq!(coils_iter.next(), Some(true)); @@ -236,7 +273,7 @@ mod tests { fn iter_over_coils() { let coils = Coils { data: &[0b0101_0011], - quantity: 3, + quantity: CoilQuantity { quantity: 3 }, }; let mut cnt = 0; for _ in coils { @@ -263,44 +300,87 @@ mod tests { #[test] fn pack_coils_into_byte_array() { - assert_eq!(pack_coils(&[], &mut []).unwrap(), 0); - assert_eq!(pack_coils(&[], &mut [0, 0]).unwrap(), 0); assert_eq!( - pack_coils(&[true; 2], &mut []).err().unwrap(), + pack_coils([], &mut []).unwrap(), + CoilQuantity { quantity: 0 } + ); + assert_eq!( + pack_coils([], &mut [0, 0]).unwrap(), + CoilQuantity { quantity: 0 } + ); + assert_eq!( + pack_coils([true; 2], &mut []).err().unwrap(), Error::BufferSize ); let buff = &mut [0]; - assert_eq!(pack_coils(&[true], buff).unwrap(), 1); + assert_eq!( + pack_coils([true], buff).unwrap(), + CoilQuantity { quantity: 1 } + ); assert_eq!(buff, &[0b_1]); let buff = &mut [0]; - assert_eq!(pack_coils(&[false], buff).unwrap(), 1); + assert_eq!( + pack_coils([false], buff).unwrap(), + CoilQuantity { quantity: 1 } + ); assert_eq!(buff, &[0b_0]); let buff = &mut [0]; - assert_eq!(pack_coils(&[true, false], buff).unwrap(), 1); + assert_eq!( + pack_coils([true, false], buff).unwrap(), + CoilQuantity { quantity: 2 } + ); assert_eq!(buff, &[0b_01]); let buff = &mut [0]; - assert_eq!(pack_coils(&[false, true], buff).unwrap(), 1); + assert_eq!( + pack_coils([false, true], buff).unwrap(), + CoilQuantity { quantity: 2 } + ); assert_eq!(buff, &[0b_10]); let buff = &mut [0]; - assert_eq!(pack_coils(&[true, true], buff).unwrap(), 1); + assert_eq!( + pack_coils([true, true], buff).unwrap(), + CoilQuantity { quantity: 2 } + ); assert_eq!(buff, &[0b_11]); let buff = &mut [0]; - assert_eq!(pack_coils(&[true; 8], buff).unwrap(), 1); + assert_eq!( + pack_coils([true; 8], buff).unwrap(), + CoilQuantity { quantity: 8 } + ); assert_eq!(buff, &[0b_1111_1111]); let buff = &mut [0]; - assert_eq!(pack_coils(&[false; 8], buff).unwrap(), 1); + assert_eq!( + pack_coils([false; 8], buff).unwrap(), + CoilQuantity { quantity: 8 } + ); assert_eq!(buff, &[0]); let buff = &mut [0, 0]; - assert_eq!(pack_coils(&[true; 9], buff).unwrap(), 2); + assert_eq!( + pack_coils([true; 9], buff).unwrap(), + CoilQuantity { quantity: 9 } + ); assert_eq!(buff, &[0xff, 1]); + + let buff = &mut [0]; + assert_eq!( + pack_coils( + [-1_i32, 1, -1, 1, 1, 1, -1, -1] + .iter() + .map(|value| value.is_positive()), + buff + ) + .unwrap(), + CoilQuantity { quantity: 8 } + ); + assert_eq!(buff, &[0b_0011_1010]); } #[test] diff --git a/src/frame/mod.rs b/src/frame/mod.rs index 875d811..4158a23 100644 --- a/src/frame/mod.rs +++ b/src/frame/mod.rs @@ -448,7 +448,7 @@ mod tests { WriteMultipleCoils( 0, Coils { - quantity: 0, + quantity: CoilQuantity { quantity: 0 }, data: &[], }, ), @@ -493,14 +493,14 @@ mod tests { let responses = &[ ( ReadCoils(Coils { - quantity: 0, + quantity: CoilQuantity { quantity: 0 }, data: &[], }), 1, ), ( ReadDiscreteInputs(Coils { - quantity: 0, + quantity: CoilQuantity { quantity: 0 }, data: &[], }), 2, From dc47791c632d3feb80f3d5a2763a246aa933127c Mon Sep 17 00:00:00 2001 From: Sebastian Scholz Date: Mon, 6 Oct 2025 15:05:45 +0200 Subject: [PATCH 2/3] Add changelog entries --- CHANGELOG.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 6f316e5..8f65807 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,9 @@ `rtu::client::decode_response`, `tcp::server::decode_response` and `tcp::client::decode_request` - Added `FrameLocation::end` helper - Fix `WriteSingleCoil` responses not including the output value +- Add `Coils::from_iter` +- Removed `modbus_core::packed_coils_len`, it is now implemented via `CoilQuantity::packed_len`. +- `modbus_core::pack_coils` now takes an iterator over coils instead of a slice of coils, and it returns a `CoilQuantity` instead of a `usize`. ## v0.2.0 (2025-09-30) From 1825858f1b5d518b6464a1ad321f99a501325ce2 Mon Sep 17 00:00:00 2001 From: Sebastian Scholz Date: Mon, 6 Oct 2025 15:09:19 +0200 Subject: [PATCH 3/3] Make the quantity public --- src/frame/coils.rs | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/frame/coils.rs b/src/frame/coils.rs index 7742fb2..dbdbb8d 100644 --- a/src/frame/coils.rs +++ b/src/frame/coils.rs @@ -113,15 +113,16 @@ pub const fn u16_coil_to_bool(coil: u16) -> Result { } } -/// A quantity of coils +/// A quantity of coils. #[cfg_attr(all(feature = "defmt", target_os = "none"), derive(defmt::Format))] #[derive(Debug, Clone, Copy, PartialEq, Eq)] pub struct CoilQuantity { - pub(crate) quantity: usize, + /// The number of coils. + pub quantity: usize, } impl CoilQuantity { - /// Calculate the number of bytes required the number of coils. + /// Calculate the number of bytes required for the number of coils. #[must_use] pub const fn packed_len(&self) -> usize { self.quantity.div_ceil(8)