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

Traits for abstract objects #5

Closed
wants to merge 1 commit into from
Closed

Conversation

flosse
Copy link
Contributor

@flosse flosse commented Nov 21, 2015

No description provided.

@flosse
Copy link
Contributor Author

flosse commented Nov 21, 2015

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
Copy link
Owner

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
Copy link
Contributor Author

flosse commented Nov 23, 2015

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
Copy link
Contributor Author

flosse commented Feb 25, 2016

Do you have any ideas for a good name?

@hirschenberger
Copy link
Owner

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
Copy link
Contributor Author

flosse commented Feb 25, 2016

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
Copy link
Contributor Author

flosse commented Feb 25, 2016

what about encapsulated instead of objects?

@hirschenberger
Copy link
Owner

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
Copy link
Contributor Author

flosse commented Feb 26, 2016

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
Copy link
Contributor Author

flosse commented Feb 29, 2016

...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
Copy link
Owner

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
Copy link
Contributor Author

flosse commented Feb 29, 2016

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
Copy link
Contributor Author

flosse commented Nov 25, 2017

I think we can close this :)

@flosse flosse closed this Nov 25, 2017
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

Successfully merging this pull request may close these issues.

None yet

2 participants