search

robotpy / robotpy-commands-v2

Python implementation of the WPILib Command Framework (2020+)
Other
2 stars 14 forks source link

Ported the ProfiledPIDSubsystem from the wpilib java source to Python #49

Closed cwstryker closed 3 months ago

cwstryker commented 3 months ago

Used ChatGPT to get 80% of the job done. This is the ProfiledPIDSubsystem only. No new tests were added. I verified functionality using a separate robot simulation project.

cwstryker commented 3 months ago

I see that some of the checks are failing. I think I understand the cause of the issue. I will work on fixing it tomorrow.

cwstryker commented 3 months ago

I fixed the bug causing the Python 3.8 and 3.9 tests to fail.

virtuald commented 3 months ago

Thanks, everything looks good.

There are multiple ProfiledPIDControllers (one for dimensionless, and one for radians), and even though they're effectively the same thing there are some C++ classes that require one or the other. If you have any experience with python's typing generics, any ideas you have to make this work with both types of ProfiledPIDControllers would be welcomed.

I posted some thoughts on a related issue at https://github.com/robotpy/robotpy-commands-v2/pull/40, but I'm still mulling it over.

If you don't have any ideas or expertise in this, that's fine. I'll contemplate and merge/adjust this once #46 is finished, which hopefully will be soon.

cwstryker commented 3 months ago

I am working on a modification to the ProfiledPIDSubsystem that is also compatible with ProfiledPIDControllerRadians objects. Since there are (currently) only two different kinds of ProfiledPIDControllers, I am just using a Union (from the typing module) to include both types of controller object. While working on the changes, mypy indicated that ProfiledPIDController.setGoal() and ProfiledPIDControllerRadians.setGoal() take different argument types. The later only accepts a float argument, where the former accepts either a TrapezoidProfile.State or a float. I think I have a fix for that, but I want to create some test code before I push up a new commit.

virtuald commented 3 months ago

I'm going to merge this without typing for now. I don't really like the union approach, and I'm too tired to think through the generic/typevar approach. We can add types back at a later date.