-
Notifications
You must be signed in to change notification settings - Fork 20
First version of iterativerobotpy.py #161
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
base: main
Are you sure you want to change the base?
Conversation
Signed-off-by: Mike Stitt <mikestitt@peas.local>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Overall this looks pretty sane.
def testExit(self): | ||
pass | ||
|
||
# todo @Deprecated(forRemoval=true, since="2025") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When we deprecate things, we typically only do it in the docstring.
wpilib.DriverStation.refreshData() | ||
self._m_watchdog.reset() | ||
|
||
self._m_word.refresh() # todo from Java implementation |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
C++ doesn't have this, I filed a bug to fix that (wpilibsuite/allwpilib#7889).
For now, just do self._word = DSControlWord()
and it should have the same effect. Leave a TODO note to switch to refresh when it gets implemented.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually, robotBase has a GetControlState method which returns (enabled, autonomous, test)... which I think would be better to use here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@auscompgeek AI tells me that "The eyes emoji (👀) generally indicates a sense of observation, attention, or interest, often with a subtle hint of intrigue or admiration. It can also be used to suggest that someone is being watched or is deliberately drawing attention to something."
What are you thinking here?
# private void printLoopOverrunMessage() { | ||
# DriverStation.reportWarning("Loop time of " + m_period + "s overrun\n", false); | ||
# } | ||
wpilib._impl.report_error.reportWarning( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, this is correct.
I would do at the top of this file from wpilib import reportWarning
. Every time there's a .
in python it's another dictionary lookup, and the rio is really slow.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some additional early feedback.
self.driverStationConnected() | ||
|
||
# If mode changed, call mode exit and entry functions | ||
if self._m_lastMode != mode: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: Python enums are singletons, so prefer using is
like the recommendation in the Python docs for enum
Top level questions: Is the filename right? Is the file in the right spot in the tree? Is the class name right? What are the appropriate tests? What happens if someone deletes the watchdog? Is the there a higher level watchdog that will stop the motors? Can somebody figure out how to call the shuffleboard? |
I'm not sure about the filename/classname yet. I think you're approaching it as an alternative, but I sorta want a replacement? But, I guess we could go either way. TBH, before deciding on alternative/replacement, probably need someone to do some testing on a real robot to see if it makes loop overrun issues more/less/same.
... yeah, the only real way to test this is likely going to be by testing examples, which is something I'd like to do but haven't taken the time to do (see #154). Java has tests for TimedRobot, but they basically do the same thing that the pyfrc test framework does (maybe that's an argument for moving some of that into here...).
If someone deletes the watchdog, that's their problem -- but also, the motor watchdog is actually a separate thing? This one just tracks loop overruns and tells the user about it.
|
Signed-off-by: Mike Stitt <mike@stitt.cc>
Signed-off-by: Mike Stitt <mike@stitt.cc>
This version simulates a TimedRobot, advances time, logs to AdvantageScope in a reasonable way, but does not move the robot in simulation. It feels like some sort of enable interlock on the simulator is missing. Generally all status results from hal.xxx calls seem to result in a strange value, but the calls seem to work.
do not seem to make the pure python files editable in place. Could use some detailed advice on how to get the python files to be editable in place. |
This is the code I'm using to simulate: https://github.com/spiresfrc9106/spiresRobot2025/blob/exp-timedrobotpy/robot.py#L46 |
For |
It's hard to tell which libraries need to be uninstalled for mostrobotpy and which need to stay for robotpy: t2.txt:
|
The error you're getting has to do with a quirk of how macOS support is implemented. What it comes down to is that you need to have all robotpy wrapper dependencies installed the same way (either via develop or installed normally). If you mix it then you sometimes get weird errors like that. Typically, I use virtualenv (and specifically, virtualenvwrapper) to manage my various python development environments. I don't recommend that normal RobotPy users use virtualenvs unless they're comfortable with them, but they're really useful for python development -- and because they're effectively disposable, you can do a lot of weird changes in one environment without worrying too much about how much you're breaking. Specifically, I have different environments set up for installation from pypi, development, and various branches that I might be working on. If you were using a virtualenv, I would just say delete your virtualenv and start installing packages from scratch. If you're not using a virtualenv, then just uninstall all robotpy-* packages and wpilib and pyntcore, and then run develop again. |
I normally use When I re-ran that in an empty My conclusion is the More basic though is that https://github.com/robotpy/mostrobotpy/blob/main/README.md should define the relationship (what they do and where they are and how to find instructions to build/install them) between pc Also more basic is that https://github.com/robotpy/mostrobotpy/blob/main/README.md should define when to run:
and
and
and the command to then the installation instructions should take one all the way to a
|
Yes and no. As mentioned above:
What this means is that if you are installing the base robotpy packages in develop mode, you must also install robotpy-rev, ctre, etc in develop mode. To install robotpy-rev, you would need to There might be a way out of the macOS problem by messing with the library name, but I haven't had time to look deeper into it. I agree there should be more documentation. I also agree the installer scripts should be better to make this easier. |
Signed-off-by: Mike Stitt <mike@stitt.cc>
…t timer math. Signed-off-by: Mike Stitt <mike@stitt.cc>
I have been trying lots of different paths to build mostrobotpy editable on OSX, Debian, and Windows, on a full production robot code base in simulation. Goal is to be able to run
on real robot code. (I'm typing not cut and pasting so there may be some typo's here)
I end up with two copies of many of the repos needed in the system:
I can uninstall those with But then I need to install a specific version of photonlibpy:
which will then also uninstalls the editable development parts of robotpy Note that I have been automating these steps here: https://github.com/MikeStitt/pythonExperiments/tree/main/tryRobotPyBuildEditable But as I search online for ways to tell This seems to be the same on debian, and osx, (I haven't got my windows build to complete yet). Thoughts on a valid development/debugging/testing installation process? -Mike |
The multiple package problem is likely caused by pip. I note that you have 23.1 installed, try upgrading. The version numbers for your editable installs are photonlibpy has a pinned dependency on Telling pip to ignore dependencies (via |
One last note, because I don't trust setuptools to do the right thing, I run We will not be using setuptools next year, so hopefully some of that will go away. |
My windows builds using x64 Native Tools Command Prompt fail this way:
|
That error seems really familiar, I think we ran into it last year? I don't use Windows at all these days. I think you need to upgrade Visual Studio... according to this CD post you need 17.9 or later? |
This feels like a silly question, really bad planning on my part. But in debian bookworm I get:
Debian bookworm doesn't support robotpy sim? |
WPILib does not support simulation on linux/aarch64 |
Actually, that doesn't seem to be true anymore, https://frcmaven.wpi.edu/ui/native/release/edu/wpi/first/halsim/halsim_gui/2025.3.2/ shows aarch64 packages. We would need to update mostrobotpy to build them and update the robotpy-meta constraints. |
I thought we do build it, it's just that by default robotpy-meta doesn't install the sim stuff on Linux aarch64? |
I have found a route to simulate a full robot with mostrobotpy editable on a OSX M3 Mac: First install Then:
Python click module does most of the heavy lifting:
Which runs these commands:
Then:
Things that were not obvious were:
|
Simulation should work fine on Linux aarch64. I've run the simulation for the examples on my Raspberry Pi 400. |
I created this issue to document where we are currently at: robotpy/robotpy-meta#33 |
Signed-off-by: Mike Stitt <mike@stitt.cc>
Getting pretty close to the final product. It feels like I've resolved most of the review comments. Adding the docstrings uncovered some new conversations we should have about consistency in the API. The remaining tasks include: resolve the remaining todo's in the code, add unit tests, perhaps functional tests, test to see how how much slower the python version is, and test as a command2 TimedCommandRobot. @virtuald @auscompgeek, please look at the todo's in the code. |
Signed-off-by: Mike Stitt <mike@stitt.cc>
print(f"Default robotPeriodic() method...Override me!") | ||
self._robotPeriodicHasRun = True | ||
|
||
# todo why is this _simulationPeriodic in previous code? Can it be simulationPeriodic and still work? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I renamed it to _simulationPeriodic
to discourage users from using it, and to put simulation stuff in physics.py
instead. See also #130
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See also the place where the physics.py is hooked in: https://github.com/robotpy/pyfrc/blob/main/pyfrc/physics/core.py#L123
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
okay changed to _simulationInit
and _simulationPeriodic
to get the physics code to work.
periodic function. | ||
""" | ||
|
||
return self.loopStartTimeUs / 1e6 # units are seconds |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think this should return seconds? The documentation says it's the same units as GetFPGATimestamp
, so it should do so?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
True. I don't know how I got lost in my translation, c++ is also microseconds: https://github.com/wpilibsuite/allwpilib/blob/main/wpilibc/src/main/native/cpp/TimedRobot.cpp#L102
Signed-off-by: Mike Stitt <mike@stitt.cc>
Signed-off-by: Mike Stitt <mike@stitt.cc>
Signed-off-by: Mike Stitt <mike@stitt.cc>
Signed-off-by: Mike Stitt <mike@stitt.cc>
Signed-off-by: Mike Stitt <mike@stitt.cc>
Signed-off-by: Mike Stitt <mike@stitt.cc>
Now that this test passes on my machine: subprojects/robotpy-wpilib/tests/test_timedrobot.py which just tests the scheduling math to schedule the future periodic callbacks, I'm fairly confident that the code is pretty close to working correctly. Perhaps @virtuald or @auscompgeek have suggestions on how to connect existing functional tests to this code? |
Well. There really aren't any python functional tests for this -- we're pretty light on testing since the wrapper stuff tends to Just Work if it compiles. The examples repo does full-robot tests -- eventually I'd like to run those tests as part of the CI for this repo, but haven't gotten around to it. It would probably make sense to port the tests in allwpilib from Java or C++ to python, assuming they exist. ChatGPT might give you a good first translation of them. |
Signed-off-by: Mike Stitt <mike@stitt.cc>
Is there a spot in the existing robotpy code base that is equivalent to this line? In the existing robotpy code base is the thread a c++ thread or a python thread? I’m hoping to not have pave new ground in the threading model of the new test. |
pyfrc likely does all the things already: https://github.com/robotpy/pyfrc/blob/main/pyfrc/test_support/controller.py |
Signed-off-by: Mike Stitt <mike@stitt.cc>
From that hint, I made a PoC that uses pytest to start to test to TImedRobotPy. On this branch: https://github.com/MikeStitt/mostrobotpy/tree/pocTimedRobotFunctionalTests I added this file: I arrived at it by deconstructing https://github.com/robotpy/pyfrc/blob/main/pyfrc/test_support/pytest_plugin.py to run the test, seeing the print statements:
Results in:
I like that the test will fit into the test infrastructure by just running I don't like that it is not very DRY in that it is essentially doing very similar things as I suspect that we could take all of the refernces to physics out of the test when testing TimedRobotPy, so the tests would stop searching for a physics.py and test robots would be defined in the test files. What do we think the best architectural way to go would be, given the existing robotpy infrastructure: Use a version of Continue to deconstruct pytest_plugin.py as something that provides fixtures and test using pytest without a plugin? Some other permutation? |
I think you're on the right track. The pyfrc tests do a lot more than what's required for these tests. In particular, there's no need for physics or commands v2 stuff, no need to run an entire match, and likely not a lot of need to do a lot of the reset-related actions that it does. What I would recommend is create fixtures that do the minimum needed to accomplish what the java tests do, and put those fixtures in |
I think the primary usefulness of the pyfrc plugin for this work is that it shows you where all the various functions are to do timing, reset, etc, so don't worry too much about DRY here since these tests and the pyfrc tests have very different purposes. |
Signed-off-by: Mike Stitt <mike@stitt.cc>
Signed-off-by: Mike Stitt <mike@stitt.cc>
Signed-off-by: Mike Stitt <mike@stitt.cc>
I seem to have lost the recipe to build mostrobotpy editable. Perhaps you can figure out what is wrong.
|
|
It appears that robotpy-build is depending on an undocumented feature of setuptools. If you downgrade setuptools that should address the issue. The next version of robotpy-build (renamed to 'semiwrap') will no longer use setuptools (and can be significantly faster too!). mostrobotpy now builds successfully with that (see #170), I expect to merge and push all of that sometime this week. |
@virtuald Thanks!. |
Signed-off-by: Mike Stitt <mike@stitt.cc>
Signed-off-by: Mike Stitt <mike@stitt.cc>
A draft to get progress going.