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

Calling the super() methods in robot.py is required for command code, but outputs a warning that they need to be overridden. #116

Closed
aidan-mundy opened this issue Feb 28, 2018 · 5 comments
Labels
command-based Command-based utilities documentation Improvements or additions to documentation

Comments

@aidan-mundy
Copy link

Ideally, this shouldn't be terrible to fix, and it isn't a real problem, but having to call the super() methods can be confusing and not doing it causes the scheduler to fail. The methods work now as such: default autonomousPeriodic/teleopPeriodic/etc = a(), your overriding version of a() = b(). Currently, a() is called by default. If a() is overridden by b(), b() is called, but then b() must call a() using super().a(). Because you always have to call the default in some way, users always receive the "override me" message. Ideally, the change should be implemented only for a commend based robot, and it would work as such: Equivalent to the old default function minus the "Override me" message = a(), function with just the "override me" message = b(), your overriding version of the old a() = c(). [note: c() actually overrides b()] By default, a() would always be called. a() runs the scheduler and other default stuff then calls b(). b() runs only if it has not been overridden by c(). If b() has been overridden, c() runs instead. Now teams with command based code would no longer have to run the super() methods, and would only receive the "override me" message when they have not overridden the default periodic method. Sorry if this seems a bit rambly, its late at night and I want to register this thought somewhere so either I or someone else can get around to fixing it later. I plan on fixing this once I finish my competition code if someone else doesn't do it first.

@aidan-mundy
Copy link
Author

For future reference, some of command code's classes are in wpilib/command, but commandBasedRobot is in robotpy-wpilib-utilities/commandbased.

@virtuald
Copy link
Member

virtuald commented Mar 2, 2018

I was wondering what you were talking about...

In general, if you're extending an object by overriding something, it's expected that you would call the base class function. I don't really see a way around it.

@auscompgeek
Copy link
Member

This is probably something that could be made clearer in our command-based docs. @amboyscout did you want to take care of this, seeing as you said you wanted to do something about it earlier this year?

@aidan-mundy
Copy link
Author

@auscompgeek I will have limited involvement with robotpy this year as I am moving to a Java based team. If I get the chance, or feel like working on this, I may still do it, but it is unlikely for at least a little while longer.

@virtuald virtuald transferred this issue from robotpy/robotpy-wpilib Dec 7, 2018
@auscompgeek auscompgeek added command-based Command-based utilities documentation Improvements or additions to documentation labels Sep 29, 2019
@auscompgeek
Copy link
Member

So the old command framework no longer exists for 2023. Is there anything we need to do now?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
command-based Command-based utilities documentation Improvements or additions to documentation
Projects
None yet
Development

No branches or pull requests

3 participants