hirschenberger / modbus-rs

Modbus implementation in pure Rust
MIT License
89 stars 22 forks source link

Traits for abstract objects #5

Closed flosse closed 6 years ago

flosse commented 8 years ago

I'd like to use these object like this:

let client = Ctx::new_with_port("127.0.0.1", 502).unwrap();
let myCoil = client.coil(0x0F3);
let res = myCoild.test().unwrap();
myCoild.toggle().unwrap();

I don't know how to implement this in a good way. Could you give me some hints? Nevertheless I started with some traits that could be used.

hirschenberger commented 8 years ago

Thank you for your contribution. When I read your PR first, I thought your approach was a little bit overengineered and I didn't see the benefit to extract extra objects for modifying and handling single coils. But after thinking deeper about it I linked the idea of a more advanced (but separated API), where it would be possible to extract a Coil object pack it in some wrapper to do automatic on and off switching by implementing the Drop trait like:

enum DropFunction {
      SwitchOn(Coil),
      SwitchOff(Coil),
      Toggle(Coil)
}

impl Drop for DropFunction {
// ...
}

let myCoil = DropFunction::SwitchOff(client.coil(0x0F3));

// coil gets switched off after leaving the current scope

BTW: I don't like the name object for this set of functionality.

flosse commented 8 years ago

BTW: I don't like the name object for this set of functionality.

I totally agree! I just had no idea for a good name ;-)

flosse commented 8 years ago

Do you have any ideas for a good name?

hirschenberger commented 8 years ago

Hmm, good question and I must confess, that I also used the word object everywhere in the scoped module documentation. So perhaps it's the natural intuitive wording.

The question is, how to merge the two functionalities of scoped and normal objects?

flosse commented 8 years ago

The question it, how to merge the two functionalities of scoped and normal objects?

I see. I did not look at the scoped objects yet.

flosse commented 8 years ago

what about encapsulated instead of objects?

hirschenberger commented 8 years ago

No, I wanted to say, that the name objects is good but how should we design the object Coil and it's specialization ScopedCoil?

struct Coil { 
       address: .... 
}
struct ScopedCoil { 
        coil: Coil, 
        fun: CoilDropFuction
}

trait Coil { 
      fn new() { ... }
      fn toggle() { ... }
}

trait ScopedCoil: Coil {
     fn new() { ... }
}

impl Drop for ScopedCoil { .... }
flosse commented 8 years ago

I'd like to do s.th. like this:

let mut client = tcp::Transport::new("192.168.0.100").unwrap();
let ro_bit = objects::RoBit::new(&mut client, 0x0120);
let rw_bit = objects::RwBit::new(&mut client, 0x0420);
match ro_bit.test() {
  Err(err) => {},
  Ok(state) => match state {
    Coil::On=> rw_bit.set(),
    Coil::Off=> rw_bit.clear()
  }
}

But that does not work, because I need a mutable reference of client multiple times :-\ So maybe holding a reference within the struct like in https://github.com/hirschenberger/modbus-rs/blob/master/src/scoped.rs#L87 is not the best way to go for the "normal" objects?

flosse commented 8 years ago

...with the current draft it would look like this:

let mut client = tcp::Transport::new("192.168.0.100").unwrap();
let ro_bit = RoBit::new(0x02);
let rw_bit = RwBit::new(0x03);
let x = ro_bit.test(&mut client).unwrap();
println!("RoBit: {:?}",x);
let y = rw_bit.test(&mut client).unwrap();
println!("RwBit: {:?}",y);
rw_bit.toggle(&mut client).unwrap();
let z = rw_bit.test(&mut client).unwrap();
println!("RwBit: {:?}",z);
hirschenberger commented 8 years ago

I think we should stay in the Modbus domain with the naming (Coil, Register...) because that's what people expect and understand when they know Modbus.

I also don't see any benefit in your OO approach to the freestanding functions for I would suggest merging the test and toggle functionality to the scoped objects so we have scoped and unscoped objects. But I don't know how to design it elegantly.

flosse commented 8 years ago

think we should stay in the Modbus domain with the naming (Coil, Register...)

I agree, I just had no other name for it to play around with ;-)

I also don't see any benefit in your OO approach to the freestanding functions

there is no real benefit as long as we don't hold a reference of the client within the objects. But we cannot do this because the reference needs to be mutable :-\

I would suggest merging the test and toggle functionality to the scoped objects so we have scoped and unscoped objects. But I don't know how to design it elegantly.

Just add it's functions?

flosse commented 6 years ago

I think we can close this :)