PileProject / drivecommand

A communication library which connects a robot (EV3, NXT, etc.) and a device (Android, PC, etc.)
Other
14 stars 4 forks source link

Proposal of Mock communicator and machine #12

Open mandaiy opened 8 years ago

mandaiy commented 8 years ago

Problem statement

Currently we do not have mock communicator; users of this library cannot use this without implementing concrete communication means.

For instance, drive cannot run program-execution on an Android emulator because Android emulators do not have the bluetooth function. For this purpose, a mocking communicator is needed.

Proposal

I'd like to explain my thoughts of the means to resolve this. I've implemented one class and one interface; MockCommunicator and MachineBackend.

MockCommunicator is a very simple communicator implementation:

MachineBackend is an interface for concrete classes of this; typical implementations are NxtBackend, Ev3Backend, and PileBackend. These concrete classes should be able to:

What do you think? Does my proposal make sense? If you have any questions and opinions particularly for the structure of these classes, feel free to leave a comment

tiwanari commented 8 years ago

Please let me confirm some stuff. They may contain some misunderstandings.

mandaiy commented 8 years ago

Is the primary reason for making MockCommunicator testing?

Well, what I really want to do with the mocks is to run the program-execution of drive on emulators. Maybe that is a kind of testing.

I'm not sure about MachineBackend. What will the class do special things in it?

The class should be able to operate the machine by calling setLineSensorState, setTouchSensorState, etc.. To my understanding there's no way to set sensors' value with existing classes because, for example, Servomotor does not store the value; it just sends real motors via communicators. So I think we have to have some intermediate classes for that purpose.


public class MockCommunicator implements ICommunicator {
    private Backend backend;

    // ...
}

public class NxtBackend implements MachineBackend {
    public setTouchSensorValue(boolean touched) { ... }
    public setLineSensorValue(int value) { ... }
    public setMotorPower(int power) { ... }
    // ...
}

// user-code

MachineBackend backend = new NxtBackend();
ICommunicator com = new MockCommunicator(backend);
// initialize machine with "com"

backend.setTouchSensorValue(10);
backend.setLineSensorValue(10);
backend.setMotorPower(10); // of course you can also set power by NxtController 

In a sense, the class is a super-controller for the machine.

I think these kind of things can be archived with MockNxtCommunicator, MockEv3Communicator and MockPileCommunicator. These classes should be able to imitate the input/output of the corresponding robot; makes state transition according to the input and returns output according to the current state.

Do you think this is needed? Which class structure do you think better?

If you still have any questions, please comment.

tiwanari commented 8 years ago

Oh, I misunderstood the purpose of these *Backend" classes... They are only used for the MockCommunicator, right?

mandaiy commented 8 years ago

They are only used for the MockCommunicator

Yes, though the classes need snazzier name...

tiwanari commented 8 years ago

Okay, now I understood and I agree MockCommunicator should be in this library not in drive.

You know, needless to say, we should keep these classes in a package whose a specific name and make some notes in the place where users can find it easily in order to tell users "they are exactly mocks" because they are somewhat confusing. These *Backend classes also should have descriptive names with Mock(?) or so.

tiwanari commented 8 years ago

I'm yet wondering when/how these backend classes are used. Maybe, this is a more complicated problem related to the design of this library.

Communication between a robot and an Android (user) is like this;

  1. A user calls a method of a device (motor/sensor) such as ColorSensor#getRgb()
  2. The device calls a method exec of *Protocol
  3. The protocol calls a method (read/write) of *Communicator
  4. The communicator sends a request to a robot
  5. The robot returns a response (bytecode)
  6. The communicator simply returns the response to the protocol
  7. The protocol interprets the response (bytecode) and returns interpreted results to the device
  8. Then the device returns the results to the user

The backend classes can simulate 4., 5. and 6. but that is somewhat nuisance because we should make these raw bytecodes... So, I think mocking protocols is easier to implement (not so easy yet).

tiwanari commented 8 years ago

However, making such backend classes is not bad. At that point, these *Backend classes are something like simulators of them, I think.

mandaiy commented 8 years ago

These *Backend classes also should have descriptive names with Mock(?) or so.

MockNxtBackend is fine to me.

The backend classes can simulate 4., 5. and 6. but that is somewhat nuisance because we should make these raw byte codes

True. But I do not want to make a breach on the existing code, so I added the backends.

Maybe creating mock protocols is an alternative way. These mock protocols should have operation functions like backends I wrote above. In that case, we have to consider how to create machine instances. Currently concrete machine classes have one constructor and whose argument is an instance of ICommunicator.

Gonna like this? though machines will know there's mocks...


interface MockProtocol { }

public class MockNxtProtocol extends ProtocolBase implements MockProtocol {
  // ...
}

public class NxtMachine {
  public NxtMachine(ICommunicator com) { /* ... */ }
  public NxtMachine(MockProtocol protocol) { /* ... */ }
}

I totally agree with you that it's gonna hard work to implement Mock*Backend. So let's discuss what kind of way is the cleanest for Mock*Protocols.

mandaiy commented 8 years ago

Ok, another idea comes to me.

public class NxtProtocol extends ProtocolBase {
  /* same as the existing code */
}

public class MockNxtProtocol extends NxtProtocol {
  // ...
}

public class NxtMachine {
  public NxtMachine(NxtProtocol protocol, ICommunicator comm)  { /* ... */ }
}
tiwanari commented 8 years ago

Yeah, indeed, in that case, there are some problems in the design of our library. Your latest suggestion looks better and we can remove icommunicator from machine constructor arguments, can't we?

mandaiy commented 8 years ago

oops, that snippet had a bug.

I updated the snippet. well, code describes better than my English.

public class NxtMachine extends MachineBase {
  // this constructor can be protected and perhaps should be.
  public NxtMachine(NxtProtocol protocol) { /* ... */ }

  public static NxtMachine createMachine(ICommunicator comm) { 
    /* effective java says static creation methods are preferable to constructors sometimes */ 
  }
}

public class MockNxtMachine extends NxtMachine {
  // surely that's it
  public MockNxtMachine() {
    super(new MockNxtProtocol());
  }

  // static method for instantiation
}

public class NxtProtocol extends ProtocolBase {
  /* same as the existing */
}

public class MockNxtProtocol extends NxtProtocol {
  // ...
}

And why not include NxtController in this library? Now we can remove android-related code in NxtController and the class can be useful for other purpose than the app. Then for mocking purpose, MockNxtControllergonna help us. That controller should have operations for state-transition as well as the same functions as normal controllers. Now the mock controllers are backends I proposed.

To sum up, what I want to create are

tiwanari commented 8 years ago

Okay, thanks for sharing your ideas 👍

In my opinion, the software library should be general enough to be used in various scenes. From this viewpoint, I have questions and opinions;

mandaiy commented 8 years ago

1st question: No.

2nd opinion: seriously there's no reference implementation of controllers in here. that's really problematic, users will get puzzled when they want to use this and modify this. so i'd suggest that, at least, one general controller should be here for each robot. that should be general enough, with which users can manipulate robots without considering too much about it and can use all IOs

tiwanari commented 8 years ago

Okay, I misunderstood. You meant some general controllers and making a simple example of them sounds helpful :D

makotoshimazu commented 8 years ago

Sorry for late coming. I'm now trying to catch up, so please wait a moment...

makotoshimazu commented 8 years ago

I've skimmed the comments now. Basically I agreed with a mock class for developers to build a testing apk. However, I'm still not sure why we need the mock classes for each model. IIUC, we now need MockMachine which implememts all methods of MachineBase.

For moving the Controllers to drivecommand, I agree with that as long as the module path is separated appropriately like examples or samples as @tiwanari mentioned.