diff --git a/docs/contents/publishing.md b/docs/contents/publishing.md index 9e8d720..47871fd 100644 --- a/docs/contents/publishing.md +++ b/docs/contents/publishing.md @@ -131,6 +131,8 @@ This ensures that system stats are published for every online robot in the fleet Robots reported offline by `_is_robot_online()` (single-robot) / `_is_fleet_robot_online(robot_id)` (fleet) are skipped, as otherwise their state would be interpreted as online. +The online check's result is also published as the robot's status on every iteration, which the Edge SDK sends only when it changed. That is what marks a robot offline in InOrbit, so its offline timestamp is stamped once, when the robot actually goes offline, and left alone afterwards. + By default, zeroed values are used. To use the connector host's actual system stats as defaults, set `publish_connector_system_stats=True` when initializing the connector. See [FleetConnector constructor](specification/connector#spec-connector-fleetconnector-constructor) for details. **Example:** diff --git a/docs/contents/specification/connector.md b/docs/contents/specification/connector.md index 176fe29..870fd8f 100644 --- a/docs/contents/specification/connector.md +++ b/docs/contents/specification/connector.md @@ -97,7 +97,11 @@ Return `None` if the map can’t be fetched. The Edge SDK uses this callback to determine if a robot should be considered online. It is invoked when InOrbit sends a `get_state` request, which happens automatically when the robot is marked as offline but system stats are still being received. Default implementation returns `True`. -The framework also calls it once per robot on every execution loop iteration, to decide whether to publish system stats at all: while it returns `False`, no system stats are published for that robot, keeping the robot offline in InOrbit. Keep the implementation cheap and non-blocking (read cached state; no network or other blocking I/O), or the connector's event loop will stall. If it raises, the framework logs a warning and publishes anyway. +The framework also calls it once per robot on every execution loop iteration. The result decides whether system stats are published for that robot — while it returns `False`, none are, keeping the robot offline in InOrbit — and is handed to the Edge SDK's `publish_status()`, which sends it only when it changed. That status message is what reports the robot offline in the first place: InOrbit requests state only from robots it already has offline, and the robot session answers InOrbit's pings for as long as the connector runs. It is also what leaves the robot's offline timestamp alone afterwards, since the status is sent once per change rather than on every iteration. + +Because the default implementation returns `True` unconditionally, a connector that does not override this method never reports any robot as offline. Override it wherever the connector can tell a robot apart from its own health — an unreachable API, a stale heartbeat, a robot missing from the fleet manager's list. + +Keep the implementation cheap and non-blocking (read cached state; no network or other blocking I/O), or the connector's event loop will stall. If it raises, the framework logs a warning and publishes anyway. ### `start()` / `join()` / `stop()` diff --git a/docs/contents/usage/fleet.md b/docs/contents/usage/fleet.md index 9e9e374..41fcdff 100644 --- a/docs/contents/usage/fleet.md +++ b/docs/contents/usage/fleet.md @@ -180,7 +180,7 @@ def _is_fleet_robot_online(self, robot_id: str) -> bool: This callback is invoked when InOrbit sends a `get_state` request, which happens automatically when the robot is marked as offline but system stats are still being received. The connector framework publishes system stats for every online robot, ensuring that any online/offline discrepancy is detected and corrected. -It is also called once per robot on every execution loop iteration, to decide whether to publish system stats at all. While it returns `False`, no system stats are published for that robot, keeping the robot offline in InOrbit. Keep it cheap and non-blocking (read cached state; no API call in the hot path), or the connector's event loop will stall. +It is also called once per robot on every execution loop iteration. While it returns `False`, no system stats are published for that robot, keeping the robot offline in InOrbit. The result is also reported to InOrbit as the robot's status, once per change — that status message is what marks the robot offline in the first place. Keep it cheap and non-blocking (read cached state; no API call in the hot path), or the connector's event loop will stall. ## Example Execution Loop diff --git a/docs/contents/usage/single-robot.md b/docs/contents/usage/single-robot.md index 5b36acd..ade5b03 100644 --- a/docs/contents/usage/single-robot.md +++ b/docs/contents/usage/single-robot.md @@ -180,7 +180,7 @@ async def _execution_loop(self) -> None: Override this method to provide custom robot health checks. The default implementation assumes the robot is online if the connector is running. This callback is invoked when InOrbit sends a `get_state` request, which happens automatically when the robot is marked as offline but system stats are still being received. -It is also called on every execution loop iteration, to decide whether to publish system stats at all. While it returns `False`, no system stats are published, keeping the robot offline in InOrbit. Keep it cheap and non-blocking (read cached state; no API call in the hot path), or the connector's event loop will stall. +It is also called on every execution loop iteration. While it returns `False`, no system stats are published, keeping the robot offline in InOrbit. The result is also reported to InOrbit as the robot's status, once per change — that status message is what marks the robot offline in the first place. Keep it cheap and non-blocking (read cached state; no API call in the hot path), or the connector's event loop will stall. ```python def _is_robot_online(self) -> bool: diff --git a/inorbit_connector/connector.py b/inorbit_connector/connector.py index 727bae1..a23c1b9 100644 --- a/inorbit_connector/connector.py +++ b/inorbit_connector/connector.py @@ -192,6 +192,10 @@ def __init__(self, config: ConnectorRootConfig, **kwargs) -> None: # controlling TLS, so True+True yields a wss:// connection). factory_kwargs["use_websockets"] = config.use_websockets self.__session_factory = RobotSessionFactory(**factory_kwargs) + # Registered on the factory, not on each session: the pool connects a session + # as soon as it builds it, and that first connection reports this callback's + # result as the robot's status. + self.__session_factory.set_online_status_callback(self._is_fleet_robot_online) # Create RobotSessionPool self.__session_pool = RobotSessionPool(self.__session_factory) @@ -481,11 +485,6 @@ def __initialize_session(self, robot_config: RobotConfig) -> RobotSession: session, path, self.__create_user_scripts_dir ) - # Set online status callback for EdgeSDK - session.set_online_status_callback( - lambda: self._is_fleet_robot_online(robot_id) - ) - # If enabled, register the provided custom commands handler if self.__register_custom_command_handler: self.__register_custom_command_handler_for_session( @@ -1106,8 +1105,9 @@ def __publish_pending_system_stats(self) -> None: """Publish stored system stats for all robots, or defaults if none stored. This method is called automatically at the end of each execution loop iteration. - For each robot in the fleet: - - If _is_fleet_robot_online(robot_id) returns False, nothing is published for + For each robot in the fleet the online status is published (the edge-sdk drops + it unless it changed), and then: + - If _is_fleet_robot_online(robot_id) returns False, no stats are published for that robot and any stats stored for it are dropped - Else if system stats were stored via publish_robot_system_stats(), those are published @@ -1122,7 +1122,8 @@ def __publish_pending_system_stats(self) -> None: Offline robots are skipped because that forced state request is answered with the robot's offline status, which refreshes its offline timestamp in InOrbit on every loop iteration. Nothing is lost: the state request only helps when the - robot is online but InOrbit believes otherwise. + robot is online but InOrbit believes otherwise, and the robot going offline is + reported directly instead, once, through the status message. """ default_values = ( self.__get_connector_system_stats() @@ -1147,10 +1148,15 @@ def __publish_pending_system_stats(self) -> None: # a broken health check keeps publishing instead of muting the robot. self._logger.warning(f"Online check failed for '{robot_id}': {e}") online = True + # Reported every iteration; the edge-sdk publishes it only when it + # changed. This is what tells InOrbit a robot went offline: it requests + # state only from robots it already has offline, and the robot session + # answers InOrbit's pings for as long as the connector runs. + session.publish_status(online) if not online: # Stats for an offline robot make InOrbit request state, and the - # offline reply refreshes the robot's offline timestamp on every - # loop iteration. Any stats stored for it are dropped. + # offline reply moves the robot's offline timestamp. Any stats + # stored for it are dropped. self._logger.debug( f"Skipping system stats publish for '{robot_id}': robot is offline" ) diff --git a/pyproject.toml b/pyproject.toml index e716ba3..b1e6b31 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -35,7 +35,7 @@ classifiers = [ "License :: OSI Approved :: MIT License", ] dependencies = [ - "inorbit-edge[telemetry]>=3.1.0,<4.0", + "inorbit-edge[telemetry]>=3.3.0,<4.0", "pydantic>=2.11,<3.0", "pydantic-settings>=2.14,<3.0", "pytz>=2025.1", @@ -45,7 +45,7 @@ dependencies = [ [project.optional-dependencies] video = [ - "inorbit-edge[video]>=3.1.0,<4.0", + "inorbit-edge[video]>=3.3.0,<4.0", ] dev = [ "bump2version~=1.0", diff --git a/tests/test_connector.py b/tests/test_connector.py index 5dda384..5c325a9 100644 --- a/tests/test_connector.py +++ b/tests/test_connector.py @@ -783,6 +783,27 @@ def boom(robot_id): hdd_usage_percentage=0.0, ) + def test_publish_pending_system_stats_publishes_online_status( + self, fleet_connector, mock_robot_session_pool + ): + """Test that each robot's online status is handed to its session. + + The edge-sdk publishes it only when it changed, and that status message is + what tells InOrbit a robot went offline: it requests state only from robots + it already has offline, and the session answers its pings while the + connector runs. + """ + fleet_connector._is_fleet_robot_online = lambda robot_id: ( + robot_id != "TestRobot2" + ) + + fleet_connector._FleetConnector__publish_pending_system_stats() + + session1 = fleet_connector._get_robot_session("TestRobot1") + session2 = fleet_connector._get_robot_session("TestRobot2") + session1.publish_status.assert_called_once_with(True) + session2.publish_status.assert_called_once_with(False) + def test_publish_connector_system_stats_uses_psutil( self, base_model, mock_robot_session_pool ): @@ -1765,7 +1786,12 @@ async def test_does_not_register_when_disabled( async def test_sets_online_status_callback( self, base_model, mock_robot_session_pool ): - """Test that online status callback is set on EdgeSDK.""" + """Test that the online status callback is set on the session factory. + + Registered on the factory rather than on each session: the pool connects a + session as soon as it builds it, so a session-level registration would come + too late for the status published on that first connection. + """ connector = Connector( "TestRobot", ConnectorRootConfig( @@ -1777,13 +1803,13 @@ async def test_sets_online_status_callback( # Initialize sessions await connector._FleetConnector__connect() - # Verify callback was set - session = connector._get_session() - session.set_online_status_callback.assert_called_once() + # Verify the callback was registered on the factory, and calls + # _is_robot_online + factory = connector._FleetConnector__session_factory + assert factory.online_status_callback == connector._is_fleet_robot_online + assert factory.online_status_callback("TestRobot") is True # True by default - # Verify the callback calls _is_robot_online - callback = session.set_online_status_callback.call_args[0][0] - assert callback() is True # Should return True by default + connector._get_session().set_online_status_callback.assert_not_called() def test_handle_command_exception_with_command_failure( self, base_model, mock_robot_session_pool