Skip to content

autodiscover opmodes + opmode example#260

Open
virtuald wants to merge 2 commits into
mainfrom
opmode-discovery
Open

autodiscover opmodes + opmode example#260
virtuald wants to merge 2 commits into
mainfrom
opmode-discovery

Conversation

@virtuald

@virtuald virtuald commented May 6, 2026

Copy link
Copy Markdown
Member

This is purely vibe-coded, but this is at least an initial look at what this will look like

@virtuald
virtuald force-pushed the opmode-discovery branch 2 times, most recently from 98427ce to cf18107 Compare May 8, 2026 12:52
Comment thread examples/robot/ExpansionHubSample/defaulttelemode.py


@overload
def autonomous(cls: _OpModeType, /) -> _OpModeType: ...

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

need docstrings for these decorators, should also document them in OpModeRobot

Comment thread subprojects/robotpy-wpilib/wpilib/opmoderobot.py

def __init__(self):
super().__init__()
_discover_decorated_opmodes(self)

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

All of the associated code with this seems a bit messy, need to reorg it a bit

@virtuald
virtuald marked this pull request as ready for review May 18, 2026 14:56
"""Provides one Gamepad per standard driver station port."""

def __init__(self):
self._gamepads = tuple(Gamepad(port) for port in range(6))

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Suggest using the constant JOYSTICK_PORTS instead of a magic number

Also, why a tuple instead of an array? I don't know that it matters but I was curious if there are benefits I don't know about.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

The AI decided it was the right fit :)

Generally, a tuple is appropriate for something that shouldn't be changed, and a list is more appropriate for things that are mutable.

Comment thread subprojects/robotpy-wpilib/wpilib/opmoderobot.py Outdated
user_controls: Any | None,
) -> dict[str, Any]:
kwargs: dict[str, Any] = {}
initializer = opmodeCls.__dict__.get("__init__")

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Is there a reason we explicitly look for __init__? Calling inspect.signature() on a type is perfectly valid, and would mean we don't have to remove the self parameter.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

If we don't grab __init__ out of the object dict, we can pass through a Callable instead. That should make the type checker happier below too.

Suggested change
opmodeCls: Callable[..., OpMode],

Comment thread subprojects/robotpy-wpilib/wpilib/opmoderobot.py Outdated
@virtuald
virtuald force-pushed the opmode-discovery branch from 7226c87 to d4937dd Compare June 7, 2026 03:50
@virtuald
virtuald force-pushed the opmode-discovery branch 2 times, most recently from 9d96931 to 0a3ce73 Compare July 16, 2026 06:59

def __init__(self):
super().__init__()
_discover_decorated_opmodes(self)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Should this be optional for testing purposes? It's slightly odd that this is defined as a module-level function rather than a method.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Yeah, I agree that the placement of the function is a bit odd. Probably should make this so it can be optional.

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.

3 participants