diff --git a/docs/contents/publishing.md b/docs/contents/publishing.md index 48c7306..9e8d720 100644 --- a/docs/contents/publishing.md +++ b/docs/contents/publishing.md @@ -123,11 +123,13 @@ publish_system_stats(**kwargs) -> None publish_robot_system_stats(robot_id: str, **kwargs) -> None ``` -Stores system stats (CPU, RAM, disk usage) to be published at the end of the execution loop. If no stats are stored for a robot during the loop iteration, default values are published automatically. +Stores system stats (CPU, RAM, disk usage) to be published at the end of the execution loop. If no stats are stored for a robot during the loop iteration, default values are published automatically. Nothing is published for a robot whose online check reports it offline (see below). All percentage values should be floats between 0.0 and 1.0 (e.g., 0.45 for 45%). -This ensures that system stats are always published for all robots in the fleet, even if the connector does not explicitly provide values. This is to ensure stability of the online status of the robot in the UI, as it forces state requests if the robot was to appear offline. +This ensures that system stats are published for every online robot in the fleet, even if the connector does not explicitly provide values. This is to ensure stability of the online status of the robot in the UI, as it forces state requests if the robot was to appear offline. + +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. 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. diff --git a/docs/contents/specification/connector.md b/docs/contents/specification/connector.md index 1975686..176fe29 100644 --- a/docs/contents/specification/connector.md +++ b/docs/contents/specification/connector.md @@ -97,6 +97,8 @@ 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. + ### `start()` / `join()` / `stop()` @@ -148,6 +150,8 @@ Publishes pose for one robot. If the `frame_id` differs from the last published If no stats are stored for a robot during the loop iteration, default values are published automatically. By default, zeroed values are used. To use the connector host's actual system stats as defaults, set `publish_connector_system_stats=True` in the [constructor](#spec-connector-fleetconnector-constructor). +Stats stored for a robot whose [`_is_fleet_robot_online()`](#spec-connector-fleetconnector-is-online) returns `False` are dropped rather than published. + If immediate publishing is required, use `_get_robot_session(robot_id)` to access the underlying `RobotSession` and call `publish_system_stats()` directly. @@ -189,7 +193,7 @@ Single-robot convenience for map fetching. The framework uses it by delegating ` **Optional override.** -Single-robot convenience for online status. The fleet-level online check delegates to this method. Called when InOrbit requests state due to a discrepancy between the robot's offline status and incoming system stats. +Single-robot convenience for online status. The fleet-level online check delegates to this method. Called when InOrbit requests state due to a discrepancy between the robot's offline status and incoming system stats, and once per execution loop iteration to decide whether to publish system stats at all. See [`_is_fleet_robot_online()`](#spec-connector-fleetconnector-is-online) for the constraints that puts on the implementation. ### Publishing wrappers @@ -207,5 +211,3 @@ Single-robot convenience for online status. The fleet-level online check delegat **Callable (advanced).** Returns the underlying Edge SDK session for the current robot. - - diff --git a/docs/contents/usage/fleet.md b/docs/contents/usage/fleet.md index 4e0113e..9e9e374 100644 --- a/docs/contents/usage/fleet.md +++ b/docs/contents/usage/fleet.md @@ -111,7 +111,7 @@ All publishing methods require a `robot_id` parameter. See the [Publishing Guide - `publish_robot_pose(robot_id, x, y, yaw, frame_id)`: Publish pose for a specific robot - `publish_robot_odometry(robot_id, **kwargs)`: Publish odometry for a specific robot - `publish_robot_key_values(robot_id, **kwargs)`: Publish key-values for a specific robot -- `publish_robot_system_stats(robot_id, **kwargs)`: Defer publishing of system stats for a specific robot; defaults are published if not called +- `publish_robot_system_stats(robot_id, **kwargs)`: Defer publishing of system stats for a specific robot; defaults are published if not called, and nothing is published while the robot is offline - `publish_robot_map(robot_id, frame_id, is_update=False)`: Publish map for a specific robot ## Background tasks @@ -178,7 +178,9 @@ def _is_fleet_robot_online(self, robot_id: str) -> bool: return self._fleet_manager.is_robot_online(robot_id) ``` -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 always publishes system stats for all robots (even zeroed defaults), ensuring that any online/offline discrepancy is detected and corrected. +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. ## Example Execution Loop diff --git a/docs/contents/usage/single-robot.md b/docs/contents/usage/single-robot.md index e6cceb7..5b36acd 100644 --- a/docs/contents/usage/single-robot.md +++ b/docs/contents/usage/single-robot.md @@ -180,6 +180,8 @@ 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. + ```python def _is_robot_online(self) -> bool: """Check if the robot is online. @@ -216,4 +218,3 @@ Scripts are automatically registered and can be executed from InOrbit. - **Simple connector**: [examples/simple-connector/connector.py](https://github.com/inorbit-ai/inorbit-connector-python/blob/main/examples/simple-connector/connector.py) - **Robot connector (CLI)**: [examples/robot-connector/](https://github.com/inorbit-ai/inorbit-connector-python/tree/main/examples/robot-connector) - **Examples index**: [examples/README.md](https://github.com/inorbit-ai/inorbit-connector-python/blob/main/examples/README.md) - diff --git a/inorbit_connector/connector.py b/inorbit_connector/connector.py index 323bb07..727bae1 100644 --- a/inorbit_connector/connector.py +++ b/inorbit_connector/connector.py @@ -1073,6 +1073,9 @@ def publish_robot_system_stats(self, robot_id: str, **kwargs) -> None: System stats are stored and published after the execution loop completes. If no stats are stored for a robot, default zeroed values are published instead. + Stats stored for a robot whose _is_fleet_robot_online() returns False are + dropped rather than published. @see __publish_pending_system_stats. + Note: If immediate publishing is required, use `_get_robot_session(robot_id)` to access the underlying RobotSession and call `publish_system_stats()` @@ -1104,8 +1107,10 @@ def __publish_pending_system_stats(self) -> None: This method is called automatically at the end of each execution loop iteration. For each robot in the fleet: - - If system stats were stored via publish_robot_system_stats(), those are - published + - If _is_fleet_robot_online(robot_id) returns False, nothing is 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 - Otherwise, default values are published (connector host stats if publish_connector_system_stats is enabled, zeroed values otherwise). @@ -1113,6 +1118,11 @@ def __publish_pending_system_stats(self) -> None: stats message is published for each robot, even if the connector does not explicitly provide values. This ensures stability of the online status of the robot in the UI, as it forces state requests if the robot was to appear offline. + + 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. """ default_values = ( self.__get_connector_system_stats() @@ -1130,10 +1140,22 @@ def __publish_pending_system_stats(self) -> None: self.__pending_system_stats = {} for robot_id, session in sessions.items(): - if pending_status := pending.get(robot_id): - session.publish_system_stats(**pending_status) - else: - session.publish_system_stats(**default_values) + try: + online = self._is_fleet_robot_online(robot_id) + except Exception as e: + # Match the edge-sdk's get_state fallback: assume online on error, so + # a broken health check keeps publishing instead of muting the robot. + self._logger.warning(f"Online check failed for '{robot_id}': {e}") + online = True + 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. + self._logger.debug( + f"Skipping system stats publish for '{robot_id}': robot is offline" + ) + continue + session.publish_system_stats(**(pending.get(robot_id) or default_values)) # Methods meant to be extended by subclasses @abstractmethod @@ -1210,7 +1232,11 @@ def _is_fleet_robot_online(self, robot_id: str) -> bool: checks (e.g., API connectivity, robot state, etc.). NOTE: State will automatically be requested from InOrbit if the robot is marked - as offline but system stats are sent. + as offline but system stats are sent. Because of that, the framework also calls + this method once per robot on every execution loop iteration to decide whether + to publish system stats at all (see __publish_pending_system_stats). Keep the + implementation cheap and non-blocking: read cached state, do not perform + network or other blocking I/O here, or the connector's event loop will stall. Args: robot_id (str): The robot ID to check @@ -1303,7 +1329,12 @@ def _is_robot_online(self) -> bool: health checks (e.g., API connectivity, robot state, etc.). NOTE: State will automatically be requested from InOrbit if the robot is marked - as offline but system stats are sent. + as offline but system stats are sent. Because of that, the framework also calls + this method on every execution loop iteration to decide whether to publish + system stats at all: while it returns False, no system stats are published for + the robot, so its offline timestamp in InOrbit stops being refreshed. Keep the + implementation cheap and non-blocking: read cached state, do not perform + network or other blocking I/O here, or the connector's event loop will stall. Returns: bool: True if robot is online, False otherwise. diff --git a/tests/test_connector.py b/tests/test_connector.py index 9f6824a..5dda384 100644 --- a/tests/test_connector.py +++ b/tests/test_connector.py @@ -736,6 +736,53 @@ def test_publish_pending_system_stats_mixed_stored_and_default( hdd_usage_percentage=0.0, ) + def test_publish_pending_system_stats_skips_offline_robots( + self, fleet_connector, mock_robot_session_pool + ): + """Test that nothing is published for robots reported offline.""" + # TestRobot2 is offline: publishing stats for it would make InOrbit request + # state and refresh its offline timestamp on every loop iteration. + fleet_connector._is_fleet_robot_online = lambda robot_id: ( + robot_id != "TestRobot2" + ) + # Stored stats must be dropped too, not just the defaults. + fleet_connector.publish_robot_system_stats( + "TestRobot2", cpu_load_percentage=0.5 + ) + + fleet_connector._FleetConnector__publish_pending_system_stats() + + session1 = fleet_connector._get_robot_session("TestRobot1") + session2 = fleet_connector._get_robot_session("TestRobot2") + session1.publish_system_stats.assert_called_once_with( + cpu_load_percentage=0.0, + ram_usage_percentage=0.0, + hdd_usage_percentage=0.0, + ) + session2.publish_system_stats.assert_not_called() + assert len(fleet_connector._FleetConnector__pending_system_stats) == 0 + + def test_publish_pending_system_stats_publishes_when_online_check_fails( + self, fleet_connector, mock_robot_session_pool + ): + """Test that a raising online check falls back to publishing.""" + + def boom(robot_id): + raise RuntimeError("health check exploded") + + fleet_connector._is_fleet_robot_online = boom + + fleet_connector._FleetConnector__publish_pending_system_stats() + + for robot_id in ("TestRobot1", "TestRobot2"): + fleet_connector._get_robot_session( + robot_id + ).publish_system_stats.assert_called_once_with( + cpu_load_percentage=0.0, + ram_usage_percentage=0.0, + hdd_usage_percentage=0.0, + ) + def test_publish_connector_system_stats_uses_psutil( self, base_model, mock_robot_session_pool ):