Fix interactive timeout callback cleanup

openpilot/selfdrive/ui/mici/layouts/onboarding.py

@@ -100,7 +100,7 @@ def __init__(self, continue_callback: Callable[[], None]):
     self._good_button = SmallCircleIconButton(gui_app.texture("icons_mici/setup/driver_monitoring/dm_check.png", 42, 42))
     self._good_button.set_touch_valid_callback(lambda: self.enabled and not self.is_dismissing)  # for nav stack
 
-    self._good_button.set_click_callback(continue_callback)
+    self._good_button.set_click_callback(lambda: self._complete(continue_callback))
     self._good_button.set_enabled(False)
 
     self._progress = FirstOrderFilter(0.0, 0.5, 1 / gui_app.target_fps)
@@ -111,13 +111,22 @@ def __init__(self, continue_callback: Callable[[], None]):
     def inactivity_callback():
       ui_state.params.put_bool("IsDriverViewEnabled", False)
 
-    device.add_interactive_timeout_callback(inactivity_callback)
+    self._inactivity_callback = inactivity_callback
 
   def show_event(self):
     super().show_event()
     self._dialog.show_event()
+    device.add_interactive_timeout_callback(self._inactivity_callback)
     self._progress.x = 0.0
 
+  def hide_event(self):
+    super().hide_event()
+    device.remove_interactive_timeout_callback(self._inactivity_callback)
+
+  def _complete(self, continue_callback: Callable[[], None]):
+    device.remove_interactive_timeout_callback(self._inactivity_callback)
+    continue_callback()
+
   def _update_state(self):
     super()._update_state()
     if device.awake and not ui_state.params.get_bool("IsDriverViewEnabled"):

openpilot/selfdrive/ui/mici/onroad/driver_camera_dialog.py

@@ -232,9 +232,15 @@ def _draw_eyes(self, rect: rl.Rectangle, driver_data):
 class DriverCameraDialog(NavWidget, BaseDriverCameraDialog):
   def __init__(self):
     super().__init__()
-    # TODO: this can grow unbounded, should be given some thought
+
+  def show_event(self):
+    super().show_event()
     device.add_interactive_timeout_callback(gui_app.pop_widget)
 
+  def hide_event(self):
+    super().hide_event()
+    device.remove_interactive_timeout_callback(gui_app.pop_widget)
+
 
 if __name__ == "__main__":
   gui_app.init_window("Driver Camera View (mici)")

openpilot/selfdrive/ui/onroad/driver_camera_dialog.py

@@ -13,12 +13,15 @@ class DriverCameraDialog(CameraView):
   def __init__(self):
     super().__init__("camerad", VisionStreamType.VISION_STREAM_DRIVER)
     self.driver_state_renderer = DriverStateRenderer()
-    # TODO: this can grow unbounded, should be given some thought
-    device.add_interactive_timeout_callback(gui_app.pop_widget)
     ui_state.params.put_bool("IsDriverViewEnabled", True, block=True)
 
+  def show_event(self):
+    super().show_event()
+    device.add_interactive_timeout_callback(gui_app.pop_widget)
+
   def hide_event(self):
     super().hide_event()
+    device.remove_interactive_timeout_callback(gui_app.pop_widget)
     ui_state.params.put_bool("IsDriverViewEnabled", False, block=True)
     self.close()
 

openpilot/selfdrive/ui/tests/test_ui_state.py

@@ -0,0 +1,40 @@
+from openpilot.selfdrive.ui.ui_state import Device
+
+
+def test_interactive_timeout_callbacks_are_unique_and_removable():
+  device = Device()
+
+  def callback():
+    pass
+
+  device.add_interactive_timeout_callback(callback)
+  device.add_interactive_timeout_callback(callback)
+  assert device._interactive_timeout_callbacks == [callback]
+
+  device.remove_interactive_timeout_callback(callback)
+  device.remove_interactive_timeout_callback(callback)
+  assert device._interactive_timeout_callbacks == []
+
+
+def test_interactive_timeout_callback_can_remove_itself(monkeypatch):
+  device = Device()
+  monkeypatch.setattr(device, "_set_awake", lambda on: None)
+
+  calls = []
+
+  def first_callback():
+    calls.append("first")
+    device.remove_interactive_timeout_callback(first_callback)
+
+  def second_callback():
+    calls.append("second")
+
+  device.add_interactive_timeout_callback(first_callback)
+  device.add_interactive_timeout_callback(second_callback)
+
+  device._interaction_time = 0
+  device._prev_timed_out = False
+  device._update_wakefulness()
+
+  assert calls == ["first", "second"]
+  assert device._interactive_timeout_callbacks == [second_callback]

openpilot/selfdrive/ui/ui_state.py

@@ -244,7 +244,12 @@ def _reset_interactive_timeout(self) -> None:
     self._interaction_time = time.monotonic() + self.interactive_timeout
 
   def add_interactive_timeout_callback(self, callback: Callable):
-    self._interactive_timeout_callbacks.append(callback)
+    if callback not in self._interactive_timeout_callbacks:
+      self._interactive_timeout_callbacks.append(callback)
+
+  def remove_interactive_timeout_callback(self, callback: Callable):
+    if callback in self._interactive_timeout_callbacks:
+      self._interactive_timeout_callbacks.remove(callback)
 
   def update(self):
     self._start_brightness_thread()  # start thread after manager forks ui
@@ -306,7 +311,7 @@ def _update_wakefulness(self):
 
     interaction_timeout = time.monotonic() > self._interaction_time
     if interaction_timeout and not self._prev_timed_out:
-      for callback in self._interactive_timeout_callbacks:
+      for callback in self._interactive_timeout_callbacks[:]:
         callback()
     self._prev_timed_out = interaction_timeout