feat: address PR review comments - blocking mode, reason tracking, README docs, package metadata

- SaveSnapshotDelayed.srv: add bool blocking and string reason fields
- ringbuffer_node.py: blocking=true waits on executor future; reason forwarded
  to _write_snapshot which writes reason.md for every snapshot route
- snapshot_helpers.py: add REASON_FILENAME, build_reason_content(), read_snapshot_reason()
- snapshot_server.py: metadata endpoint includes reason field (null when absent)
- Both package.xml: license=proprietary, maintainer=Marc Hanheide
- README.md: full documentation of delayed service, blocking mode, reason.md,
  updated ToC, Features table, Installation, Quick Start, REST endpoints table,
  and Architecture diagram
- test_delayed_snapshot.py: 22 tests covering new helpers and all service modes

Co-authored-by: marc-hanheide <1153084+marc-hanheide@users.noreply.github.com>

Author: Copilot

src/rosbag_blackbox/README.md

@@ -26,6 +26,8 @@ A ROS 2 **Humble** package that provides:
   - [QoS Settings](#qos-settings)
   - [Latched Topics](#latched-topics)
 - [Triggering a Snapshot](#triggering-a-snapshot)
+  - [Immediate trigger (blocking)](#immediate-trigger-blocking)
+  - [Delayed / non-blocking trigger](#delayed--non-blocking-trigger)
 - [Snapshot Server](#snapshot-server)
   - [Authentication](#authentication)
   - [REST Endpoints](#rest-endpoints)
@@ -49,6 +51,8 @@ A ROS 2 **Humble** package that provides:
 | **Memory efficiency** | Messages stored as raw serialised bytes; deque-based O(1) expiry |
 | **Dynamic type discovery** | Topic types resolved at runtime (like `ros2 bag record`) |
 | **On-demand snapshot** | `std_srvs/srv/Trigger` service writes the buffer to a rosbag |
+| **Delayed snapshot** | `~/save_snapshot_delayed` service: configurable pre-snapshot delay, optional blocking mode, and snapshot reason |
+| **Snapshot reason** | Every snapshot includes a `reason.md` sidecar explaining why it was taken |
 | **Auto-snapshot** | Sliding-window automatic snapshots at a configurable interval |
 | **Configurable filename** | `{datetime}`, `{date}`, `{time}`, `{timestamp}`, `{trigger}` placeholders |
 | **Node parameter snapshotting** | Collects and stores active node parameters alongside each snapshot |
@@ -69,6 +73,8 @@ A ROS 2 **Humble** package that provides:
 ```bash
 cd ~/ros2_ws
 rosdep install --from-paths src --ignore-src -r -y
+# Build the interfaces package first (provides the SaveSnapshotDelayed service)
+colcon build --packages-select rosbag_blackbox_interfaces
 colcon build --packages-select rosbag_blackbox
 source install/setup.bash
 ```
@@ -98,10 +104,16 @@ ros2 launch rosbag_blackbox rosbag_blackbox.launch.py \
 ### 2 – Trigger a snapshot
 
 ```bash
+# Immediate snapshot (blocks until written)
 ros2 service call /rosbag_blackbox/save_snapshot std_srvs/srv/Trigger
+
+# Delayed snapshot – returns immediately; snapshot taken after 10 s
+ros2 service call /rosbag_blackbox/save_snapshot_delayed \
+    rosbag_blackbox_interfaces/srv/SaveSnapshotDelayed \
+    "{delay: 10.0, blocking: false, reason: 'Anomaly detected'}"
 ```
 
-The response message contains the full path of the new rosbag directory.
+The `response.message` field contains the full path of the new rosbag directory (or a scheduling confirmation for non-blocking calls).
 
 ### 3 – Query the snapshot via REST
 
@@ -324,6 +336,8 @@ have been evicted from the time-bounded ring buffer.
 
 ## Triggering a Snapshot
 
+### Immediate trigger (blocking)
+
 Call the service exposed by the ring-buffer node:
 
 ```bash
@@ -350,6 +364,60 @@ The `response.message` field contains the absolute path of the newly created
 rosbag directory.  Snapshots triggered this way use the `manual` value for
 the `{trigger}` filename placeholder.
 
+### Delayed / non-blocking trigger
+
+The `~/save_snapshot_delayed` service accepts three optional parameters:
+
+| Field | Type | Description |
+|-------|------|-------------|
+| `delay` | `float64` | Seconds to wait before taking the snapshot (default `0`). |
+| `blocking` | `bool` | When `true`, the service call blocks until the snapshot is written. When `false` (default) it returns immediately. |
+| `reason` | `string` | Human-readable description of why the snapshot is being taken. Written to `reason.md` inside the snapshot directory. |
+
+The service always returns immediately when `blocking` is `false`, even if a
+delay is configured.  The actual snapshot is written in a background thread
+once the delay has elapsed.
+
+```bash
+# Take a snapshot after 10 s; returns immediately
+ros2 service call /rosbag_blackbox/save_snapshot_delayed \
+    rosbag_blackbox_interfaces/srv/SaveSnapshotDelayed \
+    "{delay: 10.0, blocking: false, reason: 'Detected anomaly in sensor data'}"
+
+# Take a snapshot immediately and block until it is written
+ros2 service call /rosbag_blackbox/save_snapshot_delayed \
+    rosbag_blackbox_interfaces/srv/SaveSnapshotDelayed \
+    "{delay: 0.0, blocking: true, reason: 'Manual inspection requested'}"
+```
+
+Or from Python:
+
+```python
+import rclpy
+from rclpy.node import Node
+from rosbag_blackbox_interfaces.srv import SaveSnapshotDelayed
+
+rclpy.init()
+node = Node('snapshot_client')
+cli = node.create_client(SaveSnapshotDelayed, '/rosbag_blackbox/save_snapshot_delayed')
+cli.wait_for_service()
+
+req = SaveSnapshotDelayed.Request()
+req.delay = 5.0          # wait 5 s before writing
+req.blocking = False     # return immediately
+req.reason = 'Triggered by safety monitor'
+
+future = cli.call_async(req)
+rclpy.spin_until_future_complete(node, future)
+print(future.result().message)
+```
+
+Every snapshot written through any route (immediate, delayed, or automatic)
+includes a `reason.md` sidecar file.  Automatic snapshots contain simply
+`auto`; immediate manual snapshots contain `manual trigger without specific
+reason` (unless overridden via the delayed service).  The
+[metadata endpoint](#rest-endpoints) exposes the reason as the `reason` field.
+
 ---
 
 ## Snapshot Server
@@ -395,7 +463,7 @@ containing the image as a base64-encoded data URL
 | GET | `/health` | Liveness check; returns server status and latest snapshot path |
 | GET | `/snapshots` | List all snapshot directory names |
 | GET | `/snapshots/latest` | Name of the most-recently modified snapshot |
-| GET | `/snapshots/{name}/metadata` | Topics, message counts, duration, time range |
+| GET | `/snapshots/{name}/metadata` | Topics, message counts, duration, time range, and snapshot reason |
 | GET | `/snapshots/{name}/messages` | Extract and deserialise messages (filterable) |
 | GET | `/snapshots/{name}/image/{topic}` | Nearest camera image as PNG data-URL |
 | GET | `/snapshots/{name}/laserscan/{topic}` | LaserScan rendered as top-view PNG data-URL |
@@ -550,16 +618,20 @@ from their default values.
 │     │             │  ┌────────────▼──────────────┐    │      │
 │     │             │  │  save_snapshot service    │    │      │
 │     │             │  │  (std_srvs/Trigger)       │    │      │
+│     │             │  │  save_snapshot_delayed    │    │      │
+│     │             │  │  (SaveSnapshotDelayed)    │    │      │
 │     │             │  │  auto-snapshot timer      │    │      │
 │     │             │  └────────────┬──────────────┘    │      │
 │     │             └───────────────┼──────────────────┘      │
 │     │                             │ writes rosbag            │
 │     │                             │ + node_params.yaml       │
+│     │                             │ + reason.md              │
 │     │                             ▼                          │
 │     │                    ┌────────────────┐                  │
 │     │                    │  snapshot dir  │                  │
 │     │                    │  (rosbag +     │                  │
-│     │                    │  node_params)  │                  │
+│     │                    │  node_params + │                  │
+│     │                    │  reason.md)    │                  │
 │     │                    └────────┬───────┘                  │
 │                                  │ reads                     │
 │  ┌───────────────────────────────▼───────────────────────┐   │

src/rosbag_blackbox/package.xml

@@ -8,8 +8,8 @@
     in-memory buffer and dumps them as rosbag snapshots on demand. Includes a
     FastAPI + FastMCP server that gives AI agents tools to introspect snapshots.
   </description>
-  <maintainer email="maintainer@example.com">Maintainer</maintainer>
-  <license>Apache-2.0</license>
+  <maintainer email="marc@jabas.ai">Marc Hanheide</maintainer>
+  <license>proprietary</license>
 
   <exec_depend>rclpy</exec_depend>
   <exec_depend>rcl_interfaces</exec_depend>

src/rosbag_blackbox/rosbag_blackbox/ringbuffer_node.py

@@ -72,7 +72,11 @@
 import rosbag2_py
 
 from rosbag_blackbox.ring_buffer import FileRingBuffer, RingBuffer
-from rosbag_blackbox.snapshot_helpers import format_snapshot_name, param_value_to_python
+from rosbag_blackbox.snapshot_helpers import (
+    build_reason_content,
+    format_snapshot_name,
+    param_value_to_python,
+)
 from rosbag_blackbox.topic_filter import find_matching_config, matches_pattern
 
 
@@ -588,15 +592,16 @@ def _on_save_snapshot_delayed(
         request: SaveSnapshotDelayed.Request,
         response: SaveSnapshotDelayed.Response,
     ) -> SaveSnapshotDelayed.Response:
-        """Schedule a snapshot after an optional delay, returning immediately.
+        """Schedule a snapshot after an optional delay.
 
-        The caller receives an acknowledgement right away.  The actual
-        snapshot is written by the background executor once *delay*
-        seconds have elapsed.
+        When ``request.blocking`` is ``True`` the call waits until the
+        snapshot is written before returning.  Otherwise it returns
+        immediately and the snapshot is written in the background.
 
-        :param request: Service request containing ``delay`` (float64, seconds).
+        :param request: Service request with ``delay`` (float64, seconds),
+            ``blocking`` (bool), and ``reason`` (string).
         :param response: Service response with ``success`` and ``message``.
-        :returns: Populated response indicating the snapshot was scheduled.
+        :returns: Populated response.
         """
         delay = request.delay
         if delay < 0.0:
@@ -605,7 +610,26 @@ def _on_save_snapshot_delayed(
             self.get_logger().error(response.message)
             return response
 
-        self._snapshot_executor.submit(self._write_snapshot_background, delay=delay)
+        reason: Optional[str] = request.reason if request.reason else None
+
+        future = self._snapshot_executor.submit(
+            self._write_snapshot_background, delay=delay, reason=reason
+        )
+
+        if request.blocking:
+            # Block until the background task completes.
+            # The future may re-raise any exception thrown inside
+            # _write_snapshot (e.g. ValueError for an empty buffer), so we
+            # catch broadly and surface the message to the caller.
+            try:
+                path = future.result()
+                response.success = True
+                response.message = f'Snapshot saved: {path}'
+            except Exception as exc:  # noqa: BLE001
+                response.success = False
+                response.message = f'Snapshot failed: {exc}'
+            self.get_logger().info(response.message)
+            return response
 
         if delay > 0.0:
             response.message = f'Snapshot scheduled in {delay:.3g}s'
@@ -615,24 +639,36 @@ def _on_save_snapshot_delayed(
         self.get_logger().info(response.message)
         return response
 
-    def _write_snapshot_background(self, delay: float = 0.0) -> None:
+    def _write_snapshot_background(
+        self,
+        delay: float = 0.0,
+        reason: Optional[str] = None,
+    ) -> str:
         """Write a snapshot in a background thread, optionally after a delay.
 
         :param delay: Seconds to wait before taking the snapshot.
+        :param reason: Optional human-readable explanation for the snapshot.
+        :returns: Absolute path of the created bag directory.
+        :raises: Re-raises any exception from :meth:`_write_snapshot`.
         """
         if delay > 0.0:
             time.sleep(delay)
-        try:
-            path = self._write_snapshot(trigger='manual')
-            self.get_logger().info(f'Delayed snapshot saved: {path}')
-        except Exception as exc:  # noqa: BLE001
-            self.get_logger().error(f'Delayed snapshot failed: {exc}')
+        path = self._write_snapshot(trigger='manual', reason=reason)
+        self.get_logger().info(f'Delayed snapshot saved: {path}')
+        return path
 
-    def _write_snapshot(self, trigger: str = 'manual') -> str:
+    def _write_snapshot(
+        self,
+        trigger: str = 'manual',
+        reason: Optional[str] = None,
+    ) -> str:
         """Serialise the ring buffer to a new rosbag directory.
 
         :param trigger: ``'manual'`` when called from the service,
             ``'auto'`` when called from the periodic timer.
+        :param reason: Optional human-readable explanation for why this
+            snapshot was taken.  Written to ``reason.md`` inside the bag
+            directory.  When *None* or empty a default message is used.
         :returns: Absolute path of the created bag directory.
         :raises ValueError: When the buffer is empty.
         """
@@ -698,6 +734,16 @@ def _write_snapshot(self, trigger: str = 'manual') -> str:
             f'{len(topic_types)} topics → {bag_path}'
         )
 
+        # Write reason.md sidecar ---------------------------------------------
+        reason_path = os.path.join(bag_path, 'reason.md')
+        try:
+            with open(reason_path, 'w') as fh:
+                fh.write(build_reason_content(trigger, reason))
+        except OSError as exc:
+            self.get_logger().warning(
+                f'Failed to write reason.md: {exc}'
+            )
+
         # Write node parameters sidecar if collection is configured -----------
         if self._param_patterns and self._node_params_cache:
             params_path = os.path.join(bag_path, 'node_params.yaml')

src/rosbag_blackbox/rosbag_blackbox/snapshot_helpers.py

@@ -20,12 +20,15 @@
 
 import os
 from datetime import datetime
-from typing import Any, Dict
+from typing import Any, Dict, Optional
 
 
 #: Name of the YAML sidecar file written alongside the rosbag.
 NODE_PARAMS_FILENAME = 'node_params.yaml'
 
+#: Name of the Markdown file that records why a snapshot was taken.
+REASON_FILENAME = 'reason.md'
+
 
 def format_snapshot_name(pattern: str, now: datetime, trigger: str) -> str:
     """Return a snapshot directory name from *pattern*.
@@ -109,3 +112,48 @@ def param_value_to_python(pval: Any) -> Any:
     if t == ParameterType.PARAMETER_STRING_ARRAY:
         return list(pval.string_array_value)
     return None  # PARAMETER_NOT_SET or unknown
+
+
+def build_reason_content(trigger: str, reason: Optional[str] = None) -> str:
+    """Return the content to write into ``reason.md``.
+
+    Output format depends on the inputs:
+
+    * When *reason* is provided (non-empty), returns a Markdown document::
+
+        # Reason for snapshot
+
+        {reason}
+
+    * When no *reason* is given and *trigger* is ``'auto'``, returns ``'auto\\n'``.
+    * When no *reason* is given and *trigger* is ``'manual'``, returns
+      ``'manual trigger without specific reason\\n'``.
+
+    :param trigger: ``'auto'`` or ``'manual'``.
+    :param reason:  Human-readable explanation, or ``None`` / empty string
+                    to fall back to a default message.
+    :returns: String ready to be written to disk.
+    """
+    if reason:
+        return f'# Reason for snapshot\n\n{reason}\n'
+    if trigger == 'auto':
+        return 'auto\n'
+    return 'manual trigger without specific reason\n'
+
+
+def read_snapshot_reason(bag_path: str) -> Optional[str]:
+    """Read the ``reason.md`` sidecar from *bag_path*.
+
+    :param bag_path: Path to the snapshot bag directory.
+    :returns: The raw content of ``reason.md``, or ``None`` when the file
+              does not exist.  Never raises – errors are swallowed so that
+              callers can degrade gracefully.
+    """
+    reason_file = os.path.join(bag_path, REASON_FILENAME)
+    if not os.path.exists(reason_file):
+        return None
+    try:
+        with open(reason_file) as fh:
+            return fh.read()
+    except Exception:  # noqa: BLE001
+        return None

src/rosbag_blackbox/rosbag_blackbox/snapshot_server.py

@@ -30,7 +30,8 @@
 * ``GET /snapshots/latest``
     Return the name of the most-recently modified snapshot.
 * ``GET /snapshots/{name}/metadata``
-    Full bag metadata: topics, message counts, duration, time range.
+    Full bag metadata: topics, message counts, duration, time range, and
+    snapshot reason (from ``reason.md`` if present).
 * ``GET /snapshots/{name}/messages``
     Extract and deserialise messages with optional filters.
 * ``GET /snapshots/{name}/rosout``
@@ -67,7 +68,7 @@
 from rclpy.serialization import deserialize_message
 from rosidl_runtime_py.utilities import get_message
 
-from rosbag_blackbox.snapshot_helpers import read_node_params
+from rosbag_blackbox.snapshot_helpers import read_node_params, read_snapshot_reason
 
 # ---------------------------------------------------------------------------
 # Optional deps – fail gracefully so the package still builds/imports.
@@ -950,7 +951,8 @@ def get_latest_rest() -> Dict[str, str]:
             'Object with ``path``, ``topics`` (list of recorded topics with '
             'type and serialisation format), ``topic_message_counts`` (dict '
             'mapping topic name → count), ``total_messages``, '
-            '``duration_seconds``, ``start_time_ns``, ``end_time_ns``.'
+            '``duration_seconds``, ``start_time_ns``, ``end_time_ns``, '
+            'and ``reason`` (content of ``reason.md`` if present, else ``null``).'
         ),
     )
     def get_metadata_rest(bag_name: str) -> Dict[str, Any]:
@@ -960,6 +962,10 @@ def get_metadata_rest(bag_name: str) -> Dict[str, Any]:
         each topic contains, the total recording duration, and the absolute
         start/end timestamps in nanoseconds since the Unix epoch.
 
+        The ``reason`` field contains the contents of ``reason.md`` (written
+        at snapshot time) when available, or ``null`` when the file is not
+        present.
+
         Use this before calling ``/messages`` or any render endpoint to
         confirm that the desired topic exists and to select an appropriate
         time window.
@@ -968,7 +974,9 @@ def get_metadata_rest(bag_name: str) -> Dict[str, Any]:
         if not os.path.exists(path):
             raise HTTPException(404, f'Snapshot not found: {path}')
         try:
-            return read_bag_metadata(path)
+            meta = read_bag_metadata(path)
+            meta['reason'] = read_snapshot_reason(path)
+            return meta
         except Exception as exc:  # noqa: BLE001
             raise HTTPException(500, str(exc)) from exc