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

[BUG]: wpilib.SendableChooser subscriptions not cleaned between tests #231

Open
jamesra opened this issue Feb 8, 2024 · 0 comments
Open

Comments

@jamesra
Copy link

jamesra commented Feb 8, 2024

Problem description

This bug would be resolved if issue #230 was resolved. Filing it in case there is a fast fix for this specific issue.

When running robotpy test if the user code subscribes to changes via wpilib.SendableChooser.onChange and passes a python object that references third-party hardware, such as a rev.CANSparkMax, all tests after the first fail because a reference is held to the python object across tests. Later test fail when they reinstantiate hardware that is already held by the python object retained from the last test.

From the user perspective there is no feedback from the test about why the HW instances are being created twice. Indeed, the user code has no errors and runs fine on the first try.

I was going to suggest wrapping SendableChooser in a weakref.proxy before returning it to python user code and adding a gc.collect() to PyFrcPlugin.robot(). Those changes resolved the issue in the code this was found in. However in the included reproduction code those changes did not solve the problem.

Operating System

Windows

Installed Python Packages

bcrypt                    4.1.2
cffi                      1.16.0
colorama                  0.4.6
cryptography              42.0.2
iniconfig                 2.0.0
packaging                 23.2
paramiko                  3.4.0
phoenix6                  24.1.0
Pint                      0.23
pip                       24.0
pluggy                    1.4.0
pycparser                 2.21
pyfrc                     2024.0.1
PyNaCl                    1.5.0
pynetconsole              2.0.4
pyntcore                  2024.2.1.2
pytest                    8.0.0
pytest-reraise            2.1.2
robotpy                   2024.2.1.1
robotpy-apriltag          2024.2.1.2
robotpy-cli               2024.0.0
robotpy-commands-v2       2024.2.2
robotpy-cscore            2024.2.1.2
robotpy-ctre              2024.1.1
robotpy-hal               2024.2.1.2
robotpy-halsim-ds-socket  2024.2.1.2
robotpy-halsim-gui        2024.2.1.2
robotpy-halsim-ws         2024.2.1.2
robotpy-installer         2024.1.4
robotpy-navx              2024.1.0
robotpy-pathplannerlib    2024.2.2
robotpy-playingwithfusion 2024.1.0
robotpy-rev               2024.2.0
robotpy-romi              2024.2.1.2
robotpy-wpilib-utilities  2024.0.0
robotpy-wpimath           2024.2.1.2
robotpy-wpinet            2024.2.1.2
robotpy-wpiutil           2024.2.1.2
robotpy-xrp               2024.2.1.2
setuptools                68.2.0
tomli                     2.0.1
typing_extensions         4.9.0
wheel                     0.41.2
wpilib                    2024.2.1.2

Reproducible example code

import wpilib
import rev
import weakref

def no_reference_callback(value):
    return 

class MyRobot(wpilib.TimedRobot):

    _motor: rev.CANSparkMax
    _chooser: wpilib.SendableChooser

    def robotInit(self):
        self._motor = rev.CANSparkMax(1, rev.CANSparkMax.MotorType.kBrushless)

        self._chooser = wpilib.SendableChooser()
        
        #In the code this bug was found in, a weakref.proxy combined with an extra 
        #gc.collect at the end of pyfrc.test_support.pytest_plugin.py PyFrcPlugin.robot()
        # was used to solve the bug.  It does not appear to work in this reproduction.
        #self._chooser = weakref.proxy(wpilib.SendableChooser())

        self._chooser.onChange(self._chooserChanged)
 
        # Not passing a reference to the python object in the callback will allow tests to 
        # complete normally
        # self._chooser.onChange(no_reference_callback)


    def _chooserChanged(self):
        pass
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

No branches or pull requests

1 participant