pybricks / support

Pybricks support and general discussion
MIT License
108 stars 6 forks source link

[Project] Some Initial tests with typical FLL EV3 Armature and the Motor class #19

Closed devinbreise closed 1 year ago

devinbreise commented 4 years ago

Some testing with some "medium" motors on an FLL robot using the Motor class turned up the following observations:

laurensvalk commented 4 years ago

Being able to specify the gear train is really great.

:tada: I'm really looking forward to how people will use this. I haven't seen this feature anywhere else.

Fun history note - originally we required users to specify the gear ratio. Then, for completely unrelated reasons, we had to disable floating point support on another Pybricks hub.

And this led to the integer gear values we have today, which are arguably easier to work with and without loss of generality.

it appears that one must create a new object of class Motor to change the gear ratio

Yes, that's what is intended. Mainly for technical reasons - we don't want people to change the ratio while the motor is running. Also, what would the new angle() value be after changing the ratio? But conceptually, when you swap out the mechanism, it could be argued that that is a different physical "object", which should probably start from a clean slate (angle 0 and stopped).

The run_until_stalled() method is very intriguing. This is something that the teams I have worked with tend to write in order to reliably "reset" an arm to a known (stalled) position. However, what really caught my attention was the duty_limit parameter. Occasionally I've seen teams develop very high torque armature for specific situations. These designs can be tricky to stall due to the torque they generate, so this appears to be an ideal solution.

Exactly. This is enables it to be used even with heavily geared mechanisms. Or mechanisms without gears that are just very fragile and you don't want to twist or break anything.

However, a little testing with something the kids built last season seemed to indicate I'm doing something wrong or its not actually working.

I think you've identified an issue - thanks! It is "supposed" to not work, but it should raise an error to tell you about it, and it doesn't.

And here's why: the medium motor has its default feed forward value set to 20, so you can't set the duty limit any lower. You could still do it if you lowered that first. But instead of telling you about it, the code fails and leaves it at 100. I'll fix this in the next release. :rocket:

Having the "wait" parameter on the several of the "run" methods is also a very welcome feature.

This is useful feedback, thanks. This concept stems from the NXT days, which had that as an alternative to the EV3 beam splitting. One day, we'll actually consider dropping it in favor of more generic and modern paradigms for this kind of thing, like asynchronous programming.

devinbreise commented 4 years ago

it could be argued that that is a different physical "object", which should probably start from a clean slate (angle 0 and stopped)

I've been thinking about how I would present the object model of Pybricks to a group of kids new to OO and Python. I prefer the idea that they would instantiate software objects representing the underlying hardware and then retain those objects for the duration of their program (passing them around as needed). The alternative model (create an object when ever you need to interact with the hardware) will lead to bad habits I think. Even if the Pybricks objects are essentially stateless themselves (state being stored at the ev3dev level), its unlikely other Classes the kids create or other libraries will share this trait, so I wouldn't want that less common paradigm creeping into their heads. And yes, one could argue that when swapping out armature, one "ought" to recreate the motor, but since the "motor" concept includes the hardware and software, and there is no "recreating" the hardware, it seems more natural (to me) to adjust the settings as needed on the instantiated object. Agree that resetting the gear train would reset angle() to 0 and it would be an error while the motor is not stopped, like some other settings you have. On the other hand, many teams will mess this up and end up with multiple instances of Pybricks device classes pointed at the same hardware object. Protecting them from themselves is also a good thing. Its a style thing I suppose.

the medium motor has its default feed forward value set to 20

Ahh yes! Forgot about the "F" in PDIF. Don't see that much in FIRST robotics (except for elevators) but its great that it is here. I forgot about that. When I get a chance, I'l try it with that zero'd out...

laurensvalk commented 4 years ago

We'd definitely encourage to create only one hardware instance. In fact, the new template looks like this:

#!/usr/bin/env pybricks-micropython
from pybricks.hubs import EV3Brick
from pybricks.ev3devices import (Motor, TouchSensor, ColorSensor,
                                 InfraredSensor, UltrasonicSensor, GyroSensor)
from pybricks.parameters import Port, Stop, Direction, Button, Color
from pybricks.tools import wait, StopWatch, DataLog
from pybricks.robotics import DriveBase
from pybricks.media.ev3dev import SoundFile, ImageFile

# Create your objects here
ev3 = EV3Brick()

# Write your program here
ev3.speaker.beep()

Even if the Pybricks objects are essentially stateless themselves (state being stored at the ev3dev level)

This isn't really the case. Especially the Motor and DriveBase classes are totally unique to Pybricks, independent from the ev3dev motor interfaces, which don't have most of these features or accuracy.

Swapping the gears out while the program is running is admittedly something that isn't accounted for in the existing Motor class. Note that the same holds for the positive_direction setting. Like gears, it is intentionally part of the constructor, not a runtime setting.

Put differently, Pybricks caters for a lot of use cases with the base class, but does not aim to address everything.

You could, of course, still use the class with no gears explicitly given and simply work with motor degrees. If you want to swap them dynamically and still have gears nicely abstracted in a class, this sounds like a good case to introduce OO in the classroom, and subclass it and make it your own :wink:

Ahh yes! Forgot about the "F" in PDIF. Don't see that much in FIRST robotics

LEGO kind of has it in their drivers too, but it is not configurable, and it sits in the wrong place. The medium motor has comparatively high friction, and doesn't even begin to move below approximately 20% duty, which is why it is there.

devinbreise commented 4 years ago

Pybricks caters for a lot of use cases with the base class, but does not aim to address everything.

Points taken. I would only offer this. Unless something changes in the latency for launching a program, any FLL team that uses this is very likely to create a "master control" program that will run throughout their 2.5 minutes on the board. They will also, like all FLL teams, bring the robot back for subsequent relaunches and structure their code into a number of "runs". If an equipment swap alters the gear ratio, they will face this situation. So I'm guessing it will be a fairly common scenario among Pybricks users. Personally, I suspect I would mentor a team facing that to delete the old Motor object and instantiate a new one with the new parameters (as you suggest) between "runs". Which, as you point out, is perfectly reasonable.

laurensvalk commented 4 years ago

The duty limit setter should be fixed.

Fixed by:

(these links will become public later - I'm adding these so we can easily find stuff later.)

Please see #17 to download and test the fix. Thanks again for highlighting this!

laurensvalk commented 4 years ago

Indeed, it is an advanced use case and a new Motor object will do fine. However, the advantage of Python over the graphical language is that it is much easier for power users to extend it.

I think the following 6 lines offers the functionality you want:

class GearSwapMotor(Motor):
    def __init__(self, port):
        self.port = port
        self.reconfigure(Direction.CLOCKWISE, None)

    def reconfigure(self, positive_direction, gears):
        super().__init__(self.port, positive_direction, gears)

This lets you deal with the same object the whole time, if you'd like to encourage that paradigm.

You could use it like this:

# Create your objects here
ev3 = EV3Brick()
mechanism = GearSwapMotor(Port.A)

print("Motor will rotate one rotation")
mechanism.run_target(100, 360)

wait(1000)
print("Motor will rotate 3 rotations in reverse to account for gears")
mechanism.reconfigure(Direction.COUNTERCLOCKWISE, [12, 36])
mechanism.run_target(100, 360)

It is certainly an interesting suggestion, so maybe we'll add it the reconfigure method in a future release.

devinbreise commented 4 years ago

Thanks for that awesome tip. I've worked in more programming languages than I can count over 40 years, but still fresh with Python. A subsequent call to the "constructor" on the super class on an instantiated object?! Didn't see that coming... ;)

devinbreise commented 4 years ago

I was able to re-test this. A few observations:

laurensvalk commented 4 years ago

Thanks for verifying! And yes, good points for the docs, we'll need to expand those.

So, I'll remove the bug label and turn it into a documentation issue.

laurensvalk commented 4 years ago

A subsequent call to the "constructor" on the super class on an instantiated object?! Didn't see that coming... ;)

Yeah, if I were to include it in the main Motor, I would have made a reconfigure method that is called from init instead. But (Micro)Python is pretty flexible in doing things like this.

laurensvalk commented 1 year ago

Most of the ideas discussed here have now been implemented or fixed, so we can close this.

Feel free to open a new discussion if follow up is needed.