Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Adds TrapezoidProfileCommand and TrapezoidProfileCommandRadians #40

Merged
merged 1 commit into from
Jan 24, 2024

Conversation

lospugs
Copy link
Contributor

@lospugs lospugs commented Dec 12, 2023

Commmands2. robotpy/robbotpy-commands-v2#28

Implemented using generics.

"""
A command that runs a :class:`.TrapezoidProfile`. Useful for smoothly controlling mechanism motion.

This class is provided by the NewCommands VendorDep
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't include this comment

# Defined two generic types for the Profile and ProfileState variables.
# This allows an implementation for both dimensionless and Radians
# instances of TrapezoidProfiles (or any future dimension as well)
TP = TypeVar("TP") # Generic[TrapezoidProfile]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This isn't correct. We want to add a protocol:

class TrapezoidProfile(Protocol, Generic[TS]):
    def calculate(self, t: wpimath.units.seconds, goal: TS, current: TS) -> TS:
        ...
    
    def totalTime(self) -> wpimath.units.seconds:
        ...

Then you can do

class TrapezoidProfileCommand(Command, Generic[TS]):
    """
    A command that runs a :class:`.TrapezoidProfile`. Useful for smoothly controlling mechanism motion.

    This class is provided by the NewCommands VendorDep
    """

    def __init__(
        self,
        profile: TrapezoidProfile,
        output: Callable[[TS], Any],
        getGoal: Callable[[], TS],
        getCurrent: Callable[[], TS],
        *requirements: Subsystem,
    )

And get rid of those type: ignore comments.

... I feel like the protocol should probably live in wpimath instead of here? @auscompgeek do you have an opinion on that?

... thinking about it, we probably should have protocols for anything that uses a C++ template, but that sounds obnoxious.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if it would be easier to do this with some kind of covariant/contravariant something or other instead of doing a full-blown protocol.

* robotpy#28

Co-authored-by: Dustin Spicuzza <dustin@virtualroadside.com>
@virtuald virtuald merged commit a8de5bb into robotpy:main Jan 24, 2024
18 checks passed
@virtuald
Copy link
Member

Merged without types

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants