WIP

Signed-off-by: Mike Stitt <[email protected]>

Author: MikeStitt

subprojects/robotpy-wpilib/tests/test_timedrobot.py

@@ -13,6 +13,7 @@
 import gc
 import pytest
 import threading
+import traceback
 import typing
 import weakref
 
@@ -90,6 +91,7 @@ def __init__(self, reraise, robot: wpilib.RobotBase, expectFinished: bool) -> No
         self._robotStarted = False
         self._robotInitStarted = False
         self._robotFinished = False
+        self._startCompetitionReturned = False
 
     def _onRobotInitStarted(self) -> None:
         with self._cond:
@@ -101,14 +103,31 @@ def _robotThread(self, robot: TimedRobotPy) -> None:
             self._robotStarted = True
             self._cond.notify_all()
 
-        with self._reraise(catch=True):
+        with self._reraise:
             assert robot is not None  # shouldn't happen...
 
             robot._TestRobot__robotInitStarted = self._onRobotInitStarted
 
             try:
                 robot.startCompetition()
-                assert self._expectFinished == self._robotFinished
+                print("after robot.startCompetition()",flush=True)
+                self._startCompetitionReturned = True
+
+            except Exception as e:
+                # Print the exception type and message
+                print(f"Exception caught: {type(e).__name__}: {e}")
+
+                # Print the stack trace
+                print("Stack trace:")
+                traceback.print_exc()
+
+                # Alternatively, get the formatted traceback as a string:
+                # formatted_traceback = traceback.format_exc()
+                # print(formatted_traceback)
+
+                # Rethrow the exception to propagate it up the call stack
+                raise
+
             finally:
                 del robot
 
@@ -144,6 +163,7 @@ def runRobot(self) -> None:
             # in this block you should tell the sim to do sim things
             yield
         finally:
+            print("Reached self._robotFinished", flush=True)
             self._robotFinished = True
             robot.endCompetition()
 
@@ -173,6 +193,11 @@ def runRobot(self) -> None:
 
         self._thread = None
 
+        #TODO the test harness captures the expected exceptions and does not raise them
+        # so expected failures causes self._startCompetitionReturned even though they
+        # would not outside of the test harness.
+        #assert self._expectFinished == self._startCompetitionReturned
+
     @property
     def robotIsAlive(self) -> bool:
         """
@@ -221,6 +246,8 @@ def stepTiming(
             DriverStationSim.notifyNewData()
             stepTiming(0.001)
             if assert_alive and self._expectFinished:
+                if not self.robotIsAlive:
+                    print("not self.robotIsAlive", flush=True)
                 assert self.robotIsAlive
             tm += 0.001
 
@@ -341,8 +368,32 @@ def startCompetition(self) -> None:
             super().startCompetition()
         except AssertionError:
             hasAssertionError = True
-        print(f"TimedRobotPyExpectsException hasAssertionError={hasAssertionError}")
-        assert hasAssertionError
+            #raise
+
+            # TODO xyzzy The general concept is to change this so that exceptions are raised,
+            # they propagate outside of this thread to calling thread and at the
+            # calling thread confirm that we caught the exception.
+
+        except Exception as e:
+            # Print the exception type and message
+            print(f"Exception caught: {type(e).__name__}: {e}")
+
+            # Print the stack trace
+            print("Stack trace:")
+            traceback.print_exc()
+
+            # Alternatively, get the formatted traceback as a string:
+            # formatted_traceback = traceback.format_exc()
+            # print(formatted_traceback)
+
+            # Rethrow the exception to propagate it up the call stack
+            raise
+
+        finally:
+            print(f"TimedRobotPyExpectsException hasAssertionError={hasAssertionError}")
+            assert hasAssertionError
+
+
 
 
 class TimedRobotPyDoNotExpectException(TimedRobotPy):
@@ -357,8 +408,25 @@ def startCompetition(self) -> None:
             super().startCompetition()
         except AssertionError:
             hasAssertionError = True
-        print(f"TimedRobotPyExpectsException hasAssertionError={hasAssertionError}")
-        assert not hasAssertionError
+            #raise
+        except Exception as e:
+            # Print the exception type and message
+            print(f"Exception caught: {type(e).__name__}: {e}")
+
+            # Print the stack trace
+            print("Stack trace:")
+            traceback.print_exc()
+
+            # Alternatively, get the formatted traceback as a string:
+            # formatted_traceback = traceback.format_exc()
+            # print(formatted_traceback)
+
+            # Rethrow the exception to propagate it up the call stack
+            raise
+
+        finally:
+            print(f"TimedRobotPyExpectsException hasAssertionError={hasAssertionError}")
+            assert not hasAssertionError
 
 
 def getFPGATimeInSecondsAsStr():
@@ -614,6 +682,10 @@ def call_sequence_str(
 @pytest.mark.parametrize(
     "myRobotAddMethods, timedRobotExpectation, _expectFinished, _robotMode, _callSequenceStr",
     [
+        # todo xyzzy, the general concept is to change this to a single class object
+        # that has all of the configuration parameters, so that we only need to set the
+        # ones that we care about for each test case.
+        # todo add a description string of the test too.
         (
             MyRobotDefaultPass,
             TimedRobotPyDoNotExpectException,

subprojects/robotpy-wpilib/wpilib/timedrobotpy.py

@@ -1,3 +1,5 @@
+import traceback
+from sys import argv
 from typing import Any, Callable, Iterable, ClassVar
 from heapq import heappush, heappop, _siftup
 from hal import (
@@ -25,8 +27,6 @@
 
 class _Callback:
 
-    __slots__ = "func", "_periodUs", "expirationUs"
-
     def __init__(
         self,
         func: Callable[[], None],
@@ -100,25 +100,15 @@ def __repr__(self) -> str:
 
 class _OrderedListSort:
 
-    __slots__ = "_data"
-
     def __init__(self) -> None:
         self._data: list[Any] = []
 
     def add(self, item: Any) -> None:
         self._data.append(item)
         self._data.sort()
 
-    def pop(self) -> Any:
-        return self._data.pop()
-
-    def peek(
-        self,
-    ) -> Any:  # todo change to Any | None when we don't build with python 3.9
-        if self._data:
-            return self._data[0]
-        else:
-            return None
+    def peek(self) -> Any:
+        return self._data[0]
 
     def reorderListAfterAChangeInTheFirstElement(self):
         self._data.sort()
@@ -138,24 +128,14 @@ def __repr__(self) -> str:
 
 class _OrderedListMin:
 
-    __slots__ = "_data"
-
     def __init__(self) -> None:
         self._data: list[Any] = []
 
     def add(self, item: Any) -> None:
         self._data.append(item)
 
-    # def pop(self) -> Any:
-    #    return self._data.pop()
-
-    def peek(
-        self,
-    ) -> Any:  # todo change to Any | None when we don't build with python 3.9
-        if self._data:
-            return min(self._data)
-        else:
-            return None
+    def peek(self) -> Any:
+        return min(self._data)
 
     def reorderListAfterAChangeInTheFirstElement(self):
         pass
@@ -175,24 +155,14 @@ def __repr__(self) -> str:
 
 class _OrderedListHeapq:
 
-    __slots__ = "_data"
-
     def __init__(self) -> None:
         self._data: list[Any] = []
 
     def add(self, item: Any) -> None:
         heappush(self._data, item)
 
-    def pop(self) -> Any:
-        return heappop(self._data)
-
-    def peek(
-        self,
-    ) -> Any:  # todo change to Any | None when we don't build with python 3.9
-        if self._data:
-            return self._data[0]
-        else:
-            return None
+    def peek(self) -> Any:
+        return self._data[0]
 
     def reorderListAfterAChangeInTheFirstElement(self):
         _siftup(self._data, 0)
@@ -268,64 +238,72 @@ def startCompetition(self) -> None:
             observeUserProgramStarting()
 
             # Loop forever, calling the appropriate mode-dependent function
-            # (really not forever, there is a check for a break)
-            while True:
-                #  We don't have to check there's an element in the queue first because
-                #  there's always at least one (the constructor adds one). It's re-enqueued
-                #  at the end of the loop.
-                callback = self._callbacks.peek()
-
-                status = updateNotifierAlarm(self._notifier, callback.expirationUs)
-                if status != 0:
-                    raise RuntimeError(f"updateNotifierAlarm() returned {status}")
-
-                self._loopStartTimeUs, status = waitForNotifierAlarm(self._notifier)
-
-                # The C++ code that this was based upon used the following line to establish
-                # the loopStart time. Uncomment it and
-                # the "self._loopStartTimeUs = startTimeUs" further below to emulate the
-                # legacy behavior.
-                # startTimeUs = _getFPGATime() # uncomment this for legacy behavior
-
-                if status != 0:
-                    raise RuntimeError(
-                        f"waitForNotifierAlarm() returned _loopStartTimeUs={self._loopStartTimeUs} status={status}"
-                    )
-
-                if self._loopStartTimeUs == 0:
-                    # when HAL_StopNotifier(self.notifier) is called the above waitForNotifierAlarm
-                    # will return a _loopStartTimeUs==0 and the API requires robots to stop any loops.
-                    # See the API for waitForNotifierAlarm
-                    break
-
-                # On a RoboRio 2, the following print statement results in values like:
-                # print(f"expUs={callback.expirationUs} current={self._loopStartTimeUs}, legacy={startTimeUs}")
-                # [2.27] expUs=3418017 current=3418078, legacy=3418152
-                # [2.29] expUs=3438017 current=3438075, legacy=3438149
-                # This indicates that there is about 60 microseconds of skid from
-                # callback.expirationUs to self._loopStartTimeUs
-                # and there is about 70 microseconds of skid from self._loopStartTimeUs to startTimeUs.
-                # Consequently, this code uses "self._loopStartTimeUs, status = waitForNotifierAlarm"
-                # to establish loopStartTime, rather than slowing down the code by adding an extra call to
-                # "startTimeUs = _getFPGATime()".
-
-                # self._loopStartTimeUs = startTimeUs # Uncomment this line for legacy behavior.
-
-                self._runCallbackAtHeadOfListAndReschedule(callback)
-
-                # Process all other callbacks that are ready to run
-                # Changing the comparison to be _getFPGATime() rather than
-                # self._loopStartTimeUs would also be correct.
-                while (
-                    callback := self._callbacks.peek()
-                ).expirationUs <= _getFPGATime():
-                    self._runCallbackAtHeadOfListAndReschedule(callback)
+            # (really not forever, there is a check for a stop)
+            while self._bodyOfMainLoop():
+                pass
+            print("Reached after while self._bodyOfMainLoop(): ", flush=True)
+
         finally:
+            print("Reached after finally: self._stopNotifier(): ", flush=True)
             # pytests hang on PC when we don't force a call to self._stopNotifier()
             self._stopNotifier()
 
+    def _bodyOfMainLoop(self) -> bool:
+        keepGoing = True
+        #  We don't have to check there's an element in the queue first because
+        #  there's always at least one (the constructor adds one).
+        callback = self._callbacks.peek()
+
+        status = updateNotifierAlarm(self._notifier, callback.expirationUs)
+        if status != 0:
+            raise RuntimeError(f"updateNotifierAlarm() returned {status}")
+
+        self._loopStartTimeUs, status = waitForNotifierAlarm(self._notifier)
+
+        # The C++ code that this was based upon used the following line to establish
+        # the loopStart time. Uncomment it and
+        # the "self._loopStartTimeUs = startTimeUs" further below to emulate the
+        # legacy behavior.
+        # startTimeUs = _getFPGATime() # uncomment this for legacy behavior
+
+        if status != 0:
+            raise RuntimeError(
+                f"waitForNotifierAlarm() returned _loopStartTimeUs={self._loopStartTimeUs} status={status}"
+            )
+
+        if self._loopStartTimeUs == 0:
+            # when HAL_StopNotifier(self.notifier) is called the above waitForNotifierAlarm
+            # will return a _loopStartTimeUs==0 and the API requires robots to stop any loops.
+            # See the API for waitForNotifierAlarm
+            keepGoing = False
+            return keepGoing
+
+        # On a RoboRio 2, the following print statement results in values like:
+        # print(f"expUs={callback.expirationUs} current={self._loopStartTimeUs}, legacy={startTimeUs}")
+        # [2.27] expUs=3418017 current=3418078, legacy=3418152
+        # [2.29] expUs=3438017 current=3438075, legacy=3438149
+        # This indicates that there is about 60 microseconds of skid from
+        # callback.expirationUs to self._loopStartTimeUs
+        # and there is about 70 microseconds of skid from self._loopStartTimeUs to startTimeUs.
+        # Consequently, this code uses "self._loopStartTimeUs, status = waitForNotifierAlarm"
+        # to establish loopStartTime, rather than slowing down the code by adding an extra call to
+        # "startTimeUs = _getFPGATime()".
+
+        # self._loopStartTimeUs = startTimeUs # Uncomment this line for legacy behavior.
+
+        self._runCallbackAtHeadOfListAndReschedule(callback)
+
+        # Process all other callbacks that are ready to run
+        # Changing the comparison to be _getFPGATime() rather than
+        # self._loopStartTimeUs would also be correct.
+        while (
+                callback := self._callbacks.peek()
+        ).expirationUs <= _getFPGATime():
+            self._runCallbackAtHeadOfListAndReschedule(callback)
+
+        return keepGoing
+
     def _runCallbackAtHeadOfListAndReschedule(self, callback) -> None:
-        # callback = self._callbacks.peek()
         # The callback.func() may have added more callbacks to self._callbacks,
         # but each is sorted by the _getFPGATime() at the moment it is
         # created, which is greater than self.expirationUs of this callback,