Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Proposal: Functions for dealing with multi-register values #33

Open
groffta opened this issue Oct 15, 2021 · 4 comments
Open

Proposal: Functions for dealing with multi-register values #33

groffta opened this issue Oct 15, 2021 · 4 comments

Comments

@groffta
Copy link
Contributor

groffta commented Oct 15, 2021

Commonly, Modbus utilizes values that are represented by multiple registers (IEEE 32-bit Float being the most common). Would there be any interest in including this higher-level functionality? If so, what thoughts are there on how to implement this? I've accomplished this in my own projects by implementing traits on f32 and u32 that provide T::to_registers(self) -> [u16;2] and T::from_registers([u16;2]) -> T. For a more general implementation, there would need to be a way to specify either low or high word order. Any input here is appreciated.

@hirschenberger
Copy link
Owner

I also thought about a more descriptive and high-level interface. Dreaming of a generically parameterized trait that statically describe the process image. In C++ this would be possible using some template-metaprograming. Rust is still a little limited in that regard and tbh. I'm also no Rust guru (but maybe a good opportunity to learn). I think the biggest drawback is that rust doesn't have variadic generics so maybe some macro-magic is required here.

The goal would be to let the compiler validate data-access at compiletime to find type and index errors and to access a static stack array efficiently.

@groffta
Copy link
Contributor Author

groffta commented Oct 18, 2021

I'm not sure you would need that level of abstraction in the modbus library. It feels like that level of validation should be left to device driver implementations. That being said, you could add the following type agnostic functions as a QoL improvement:

Transport::read_holding_value<T: ModbusValue>(&mut self, start_addr: u16) -> Result<T>;
Transport::read_input_value<T: ModbusValue>(&mut self, start_addr: u16) -> Result<T>;
Transport::write_value<T: ModbusValue>(&mut self, start_addr: u16, val: T) -> Result<()>;

pub trait ModbusValue {
  fn to_registers(self) -> Vec<u16>;
  fn from_registers(registers: Vec<u16>) -> Option<Self>;
  fn word_count() -> usize;
}

impl ModbusValue for u8 { ... }
impl ModbusValue for u16 { ... }
impl ModbusValue for u32 { ... }
impl ModbusValue for u64 { ... }
impl ModbusValue for i8 { ... }
impl ModbusValue for i16 { ... }
impl ModbusValue for i32 { ... }
impl ModbusValue for i64 { ... }
impl ModbusValue for f32 { ... }
impl ModbusValue for f64 { ... }

And add the following to deal with word order:

pub enum WordOrder {
  LowHigh,
  HighLow,
}

Transport::get_word_order(&self) -> WordOrder;
Transport::set_word_order(&mut self, order: WordOrder);

I'm willing to implement this if you don't have any objections. What are your thoughts?

@hirschenberger
Copy link
Owner

hirschenberger commented Oct 19, 2021

LGTM

Would it make sense to also add Coils to ModbusValue or even implement ModbusValue for Vec<Coil>?

@groffta
Copy link
Contributor Author

groffta commented Oct 19, 2021

On the surface, it looks like Coils should be included, but since they work on the bit level, and have asymmetric data representation depending on what function code you are using, I think it makes more sense to leave them as their own explicit functions.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants