Dual motor control in Smartcar lib

[samuk]

Description

The existing Smart car Lib is designed for a single-motor driver: https://github.com/rosmo-robot/smartcar_shield/tree/master/src/motor This robot has two motor drivers.

See also https://github.com/adityakamath/akros2_firmware/tree/akros2_galactic/src/motor for dual motor implementation using micro_ros_arduino

Definition of Done

It is possible to control two TB6612FNG using the code in this repo

[platisd]

How many different motors are there in total? Is it two on each driver?

[platisd]

Anyway, unless I have misunderstood you, the linked pull request should solve this issue and it makes sense to be supported by smartcar_lib.

I can merge after you give some feedback whether this is what you wanted.

[samuk]

Yes, that's correct two motors per driver. Thanks for the pull! It looks like this code would support a skid steer use case where the motors on the left or right side of the robots move simultaneously. It's great to have this, thanks

It looks like wouldn't yet support a mecanum use case, where each of the four motors can move in an arbitrary direction, perhaps @adityakamath can express the requirements here more clearly than I can.

[platisd]

The mecanum use case would fall under a different/new way of "steering". But as long as we have all the other building blocks together it shouldn't be an issue to fit right in along with the rest of the library.

I could consider supporting the mecanum as a way of steering the car on the upstream library as long as it's not overly complex or disruptive.

[samuk]

Great stuff. Maybe there's something in how the Linorobot2 codebase handles switching that's useful to emulate/copy?

[platisd]

Not sure, what are we looking for?

[adityakamath]

Hey @platisd I had a look at the PR and it looks good to me! I think with this, adding mecanum control should be quite straightforward IMO, especially in the open loop case.

For closed loop, with an encoder on each wheel/motor, I'm not sure how disruptive the changes would be. Are 4 encoders supported right now?

[platisd]

Thinking of it more carefully, the mecanum case may actually not be able to satisfy the Control interface. If I've understood it right with mecanum you can move sideways etc, which is not modeled in the current design of the library. So I'm taking back what I said before. Integrating it with the rest is not trivial at all as it's too different compared to the current ways of steering.

To put it simply, you can't fully control a mecanum car with setSpeed and setAngle as you do with a conventional car.

[adityakamath]

Can I propose updating the interface by splitting setSpeed into setXSpeed and setYSpeed? It would still work with the existing drive configs (by setting Y speed to 0). Then, controlling a mecanum car should be possible.

[platisd]

Can I propose updating the interface by splitting setSpeed into setXSpeed and setYSpeed? It would still work with the existing drive configs (by setting Y speed to 0). Then, controlling a mecanum car should be possible.

@adityakamath To begin with, updating an interface and adding some functions that don't make sense all its users (i.e. the "conventional" cars) is generally considered a bad idea. If we are adding setXSpeed and setYSpeed methods then these definitely belong in a different interface. That's something to consider in the future though, look at point (3) later on.

But you made me start thinking more. :sweat_smile: :bulb:

The Control interface also exposes the overrideMotorSpeed function which allows for "advanced" users to have more accurate control over the output towards the motors. I can consider renaming the function to something that makes sense for all the use-cases. We can be creative with the name, so I don't think that will be an issue. It's also good the existing Car classes don't use the particular method other than relaying the arguments passed to it to the Control interface. It won't disrupt anything that currently works in other words.

To sum up my thoughts and conclude:

1. Could we have a mecanum car, driven like a conventional car (and get all the existing functionality for "free") if we sacrifice the side-ways moving functionality and set one of the axis' speed to 0? From the user's perspective if would look something like:

   
   MecanumControl control{motor1, motor2, motor3, motor4}; // There are 4 motors right?
   SimpleCar car{control};

car.setSpeed(50); // Move only towards one axis "forward" // Turn like a conventional car/tank car.setAngle(90); // No speed on "one side of the car", 50% speed on the other side

2. Then if the user wants to move side ways:
```cpp
// Consider we may rename `overrideMotorSpeed` to something better
car.overrideMotorSpeed(50, 100); // 50% x-speed, 100% y-speed

3. In the long run,there should probably be a set of MecanumCar classes, i.e. SimpleMecanumCar, DistanceMecanumCar, SmartMecanumCar which should (not?) have any setSpeed and setAngle methods. But let's keep that in the back of our heads and not discuss further until we must.

@samuk @adityakamath Do you think using a mecanum car as described in (1) and (2) is acceptable for the users?

[adityakamath]

Not sure how acceptable it will be for users, but it definitely is a start for providing mecanum support.

@samuk I think the goal should be to first provide micro ROS support to the existing configurations, and eventually after some brainstorming, we can add the mecanum support as well.

[samuk]

Great, that sounds like a good approach. Getting the skid steer working on the hardware is likely to be a bit of work in itself.

So I think the proposed pull would resolve this ticket. Thanks, @platisd