robotpy / pyfrc

python3 library designed to make developing RobotPy-based code easier!
MIT License
50 stars 35 forks source link

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

Open jamesra opened 7 months ago

jamesra commented 7 months ago

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