WasabiFan
commented
5 years ago On the topic of combining classes...
I see two main problems with the architecture of our drive classes as it stands now.
- It's hard/unnatural to compose actions of multiple different drive styles. If one drive action is more natural as a tank operation and the next one is better as steering, I must create two drive objects and switch between them. Similarly for the new differential drive class. This isn't a huge issue, but I certainly think it's worth considering.
- It isn't obvious which parameters are which. Even with using our speed unit classes, something like this is just confusing:
steering_drive.on_for_rotations(-20, SpeedPercent(75), 10)I have to look up the documentation every time to figure out which parameters are speeds, which are steerings, and which are distances.
I'm trying to figure out if there is a sane way to improve on this. With the existing library my best suggestion to solve the second point is to include the keyword argument names, but that is something one must manually choose to do and I doubt it's common.
Here are some of my thoughts...
I think it would make a lot of sense to try to combine these classes into a generic "drive" class. That would at least solve the first point above.
Thinking more outside the box:
What if we could provide something like this?
drive = RobotDrive(OUTPUT_A, OUTPUT_B)
drive.with_steering(25).for_seconds(10)
drive.with_tank(50, 75).for_degrees(90)The goal here would be to introduce some natural language so that a) there's no ambiguity about what each value does and b) it's natural to compose different types of operations. Some points of interest:
- The way I imagined this working above would mandate that the "quantity" (time/distance/etc.) comes after the operation specifier. That could be flipped, or perhaps made so that both are setters and there's a final
.run()or similar that must be called. - This style could be extended. If we thought of "with_steering" and friends as setters on a chained operation object (the precise class design would need some thought) then you might be able to have additional setters to configure non-blocking operations and stop actions. I'm not sure how useful/clear that would be.
- It would be really nice to have differential drive in here too. Sadly, that requires additional configuration. Maybe those parameters could be optionally passed to the constructor of this shared object? Or have a setter?
I truly don't want to be disruptive unless we really are sure it is a major improvement -- at this point, I'm sure many people are using our existing interfaces. Nonetheless, an interface like this feels like a significant improvement to me. It models the same EV3-G blocks as before but treats it like a natural Python construct. This would require further discussion.
Some other difficulties:
- Are there any perf issues that would need to be considered here? Function calls and object instantiations aren't free on our low-power hardware.
- Would this be a detriment to documentability? Depending on the concrete implementation, this interface might be confusing when looking at a method reference.
dwalton76
bpmerkel
There was some discussion about this in #478
We now have several classes in motor.py that are dealing with driving two wheeled robots:
These could all be moved to a drive.py file to provide some cleaner organization of code. Do we also need to move the Speed classes?
While we are at it we should look into combining some of these classes. MoveTank and MoveSteering may be separate because EV3-G does it that way? MoveDifferential could be moved into MoveTank but certain methods would only work if the wheel object is created and the wheel_distance_mm specified.
MoveJoystick is unique enough we should probably leave it on its own.