Enhance documentation and API descriptions for FIREQ-Client

[f-riente]

Added client documentation and fixed triggering from components to FIREQ-docs

[Copilot]

Pull request overview

This PR adds initial Sphinx/MyST documentation for FIREQ-Client (overview, install, usage, API, and usage subpages) and introduces a GitHub Actions workflow intended to trigger a rebuild of the central FIREQ-docs site when client docs change.

Changes:

• Added new MyST Markdown documentation pages under docs/ (overview, installation, API, and detailed usage pages).
• Documented the interactive client/plotter workflows (commands, experiment definition YAML, output layout, plotting/export).
• Added a GitHub Actions workflow to dispatch an event to vlsi-nanocomputing/FIREQ-docs on documentation updates.

Reviewed changes

Copilot reviewed 11 out of 12 changed files in this pull request and generated 13 comments.

Show a summary per file

File	Description
other/init.py	Empty package initializer (no functional behavior added).
docs/index.md	Adds docs landing page + toctree.
docs/overview.md	Adds high-level client overview and concepts.
docs/installation.md	Adds local installation + run instructions.
docs/usage.md	Adds usage entry page + toctree.
docs/api.md	Adds architecture narrative + autosummary stubs.
docs/usage/command.md	Documents interactive client commands.
docs/usage/experiment_definition.md	Documents YAML experiment definition structure + example.
docs/usage/output.md	Documents experiment output directory/file structure.
docs/usage/plot.md	Documents plotter-based plotting/export workflows.
.github/workflows/trigger-fireq-docs.yml	Adds workflow to dispatch a rebuild event to FIREQ-docs.

Suppressed comments (4)

docs/usage/command.md:31

• set_nyquist is documented as an interactive command, but it is not available in the client’s command completer/dispatcher. Keeping it here will mislead users.

| Command | Arguments | Description |
| :--- | :--- | :--- |
| `mts_sync` | — | Triggers Multi-Tile Synchronization (MTS) across RF-DC converter tiles to align sample clocks. |
| `set_nyquist` | `<tile> <block_id> <zone>` | Configures the Nyquist operating zone for a target converter block.<br>_Example:_ `set_nyquist 228 0 2` sets the 2nd Nyquist zone on tile 228, block 0. |

docs/api.md:70

• The eval-rst block needs a closing fence at the end of the file; otherwise the directive never terminates and the page won’t render correctly.

   other.data_fetcher.ThreadedSocketReader

docs/usage.md:23

• The {toctree} block needs a closing fence after the last entry so the page renders correctly.

usage/plot

docs/usage/experiment_definition.md:216

• These steps should be renumbered after fixing the missing step 3 so the list stays sequential.

5. wait for the acquisition to complete,
6. inspect the generated output files.

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[Copilot]

Copilot was unable to review this pull request because the user who requested the review is ineligible. To be eligible to request a review, you need a paid Copilot license, or your organization must enable Copilot code review.

[Copilot]

Copilot was unable to review this pull request because the user who requested the review is ineligible. To be eligible to request a review, you need a paid Copilot license, or your organization must enable Copilot code review.

[Copilot]

Copilot was unable to review this pull request because the user who requested the review is ineligible. To be eligible to request a review, you need a paid Copilot license, or your organization must enable Copilot code review.

[Copilot]

Copilot was unable to review this pull request because the user who requested the review is ineligible. To be eligible to request a review, you need a paid Copilot license, or your organization must enable Copilot code review.

[Copilot]

🔵 Needs a closer look

The central API reference cannot render as configured, and dispatched builds can use the wrong client branch.

Review details

Suppressed comments (11)

Previously missed (6) — in code that hasn't changed since the last review.

docs/api.md:36

• This API reference will not render in the central documentation build. FIREQ-docs does not enable sphinx.ext.autosummary/autodoc, add the checked-out client to sys.path, or install/mock its import dependencies, so these directives and imports cannot be resolved. Configure the central Sphinx build accordingly or replace this block with static API documentation.

```{eval-rst}
.. rubric:: Core Client & CLI

.. autosummary::

docs/api.md:25

• The runtime does not render plots while receiving data: Client only saves the files, and plotting/export require a separate python plotter.py session. Describing plotting as part of this lifecycle gives users the wrong expectation after run_yaml.

5. **Data Streaming & Export**: Listens for inbound binary DMA frames, converts complex IQ payloads into structured data, and saves them locally as JSON/Pickle files. The client `export` command creates the single-`data.pkl` format consumed by the separate `FIREQ_PLOTTER` package.

docs/usage/command.md:11

• ping does not measure or report latency; _ping() only sends the request and prints the returned header. Remove the latency claim unless timing is added to the implementation.

| `ping` | Sends a ping request and prints the server response. |

docs/usage/plot.md:21

• plot_2d has no phase mode: its CLI options select magnitude, real, imaginary, or real+imaginary values. Advertising phase here directs users to unsupported behavior (phase is available only in plot_3d_heat).

command first.

docs/usage/experiment_definition.md:37

• This marks _name as required in user YAML, but acquisition nodes are addressed by their /axisAcquisitionIP_* path and every shipped experiment—including this page's complete example—omits this field. Label it internal rather than instructing users to supply it.

This issue also appears in the following locations of the same file:

• line 56
• line 111

| `_name` | `str` | Yes | Name of the acquisition node instance |

docs/api.md:11

• The sockets are not configured as non-blocking: socket.create_connection() retains blocking mode, while recv() and sendmsg() run on background threads. Calling this a non-blocking TCP session misstates the architecture; the concurrency comes from threaded I/O.

1. **Connection Layer**: Establishes and manages non-blocking TCP socket sessions with the server.

.github/workflows/trigger-fireq-docs.yml:30

• The receiving FIREQ-docs workflow does not read this payload: its checkout always uses the client ref pinned in fireq-release.yaml (docs/initial_doc). Because this workflow accepts every docs/** branch (including the existing docs/v0.1.0-init), such pushes dispatch successfully but rebuild documentation from the wrong branch. The receiver needs to consume the repository/ref/sha payload, or this trigger must be restricted to the manifest-pinned branch.

            -d "{\"event_type\":\"fireq-component-docs-updated\",\"client_payload\":{\"repository\":\"${SOURCE_REPO}\",\"ref\":\"${SOURCE_REF}\",\"sha\":\"${SOURCE_SHA}\"}}"

docs/usage/output.md:25

• The data_99_99 endpoint is valid only when both sweep variables contain 100 values. As written, the paragraph applies this example to any two-variable sweep even though each index stops at its own variable's num - 1.

- ...

docs/usage/experiment_definition.md:216

• The sequence skips step 4, making the execution instructions incorrectly numbered.

5. wait for the acquisition to complete,
6. inspect the generated output files.

docs/usage/experiment_definition.md:56

• This marks _name as required in user YAML, but generator nodes are addressed by their /axisGeneratorIP_* path and every shipped experiment—including this page's complete example—omits this field. Label it internal rather than instructing users to supply it.

| `_name` | `str` | Yes | Name of the signal generator instance |

docs/usage/experiment_definition.md:111

• This marks _name as required in user YAML, but trigger nodes are addressed by their /axisTriggerGenerator_* path and every shipped experiment—including this page's complete example—omits this field. Label it internal rather than instructing users to supply it.

| `_name` | `str` | Yes | Name of the trigger generator instance |

• Files reviewed: 12/13 changed files
• Comments generated: 0 new
• Review effort level: Balanced

[Copilot]

🟡 Changes recommended

Several instructions describe unsupported behavior or omit required configuration.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

Suppressed comments (1)

docs/usage/experiment_definition.md:216

• The ordered procedure skips step 4, which makes the execution sequence confusing.

5. wait for the acquisition to complete,
6. inspect the generated output files.

• Files reviewed: 12/13 changed files
• Comments generated: 4
• Review effort level: Balanced

[Copilot]

🔵 Needs a closer look

Several instructions reference nonexistent configuration constants, paths, or unsupported plotting behavior.

Review details

Suppressed comments (13)

Previously missed (9) — in code that hasn't changed since the last review.

docs/usage/experiment_definition.md:9

• The repository has no experiments/ directory; its bundled definitions are under yaml_experiment_configurations_examples/ (also documented in README.md:41,97-100). Point readers at the directory that actually exists.

Experiment definitions are stored in YAML format (typically within the `experiments/` directory) and structured into three primary top-level sections:

docs/usage/experiment_definition.md:212

• This copyable command refers to the nonexistent experiments/ directory, so it will fail for every bundled example. Use the repository's actual example directory.

run_yaml experiments/<experiment_name>.yaml

docs/index.md:41

• The contribution guide points to experiments/, but this repository stores its examples in yaml_experiment_configurations_examples/. The current path sends contributors to a directory that does not exist.

- adding or updating examples in `experiments/`;

docs/usage/plot.md:21

• This statement is too broad: plot_2d rejects a non-swept accumulated experiment and explicitly directs the user to plot_iq (FIREQ_PLOTTER/plotting/plot_2d.py:46-52). Qualify the direct plotting path by output mode.

Visualizes standard $Y(X)$ response curves across parameter sweeps. The input
directory must contain `data.pkl`. Non-swept experiments can be plotted
directly after `run_yaml`; swept experiments require the client's `export`
command first.

docs/usage/plot.md:7

• “Slicers” is inconsistent with the slider controls described and implemented elsewhere on this page.

| `plot_2d` | 1D / Multi-D (with Slicers) | Experiment directory with `data.pkl` | Standard line plots for 1D sweeps or multi-sweep data with dynamic slice controls. |

docs/api.md:3

• The client does not use non-blocking sockets: socket.create_connection creates a blocking socket and blocking recv/sendmsg calls run in background worker threads (client.py:49-53, receive_worker.py:70-80). Describe threaded I/O rather than non-blocking TCP in both assertions.

Welcome to the `FIREQ-Client` API reference and architecture guide. The client acts as the front-end workstation interface that connects to the `FIREQ-Server`, processes experiment definitions, manages non-blocking TCP streams, and visualizes acquisition results.

docs/api.md:80

• These entries name modules, not the plotting functions, whose actual definitions are the underscored callables inside those modules. As written, the promised function signatures are not included in the autosummary.

   FIREQ_PLOTTER.plotting.plot_2d
   FIREQ_PLOTTER.plotting.plot_3d_heatmap
   FIREQ_PLOTTER.plotting.plot_iq
   FIREQ_PLOTTER.plotting.plot_spectr

docs/api.md:12

• Remove the duplicated space between “framing” and “and.”

2. **Protocol & Networking Layer**: Implements length-prefixed binary framing  and manages background I/O threads.

docs/usage/plot.md:10

• plot_spectr hard-codes the first sweep axis as a frequency and labels the maximum-distance point “Frequency” (plot_spectr.py:71-79), so advertising power sweeps produces a misleading result.

| `plot_spectr` | 1D Frequency / Power Sweeps | Two experiment directories with `data.pkl` | Overlaid response curves comparing transmission/dispersion shifts between two states. |

docs/usage/plot.md:24

• plot_2d does not offer a phase mode: its accepted options are magnitude, real, imaginary, or real+imaginary (FIREQ_PLOTTER/plotter.py:25,128-135). Advertising phase here gives users an unsupported plotting choice.

* **Single Sweep Variable:** Generates a 2D line plot mapping the measured IQ magnitude, phase, or quadrature components against the swept parameter.

docs/usage/experiment_definition.md:216

• The procedure skips step 4, making the ordered workflow inconsistent.

5. wait for the acquisition to complete,
6. inspect the generated output files.

docs/installation.md:29

• These setup instructions cannot be followed: run_client.py defines no SERVER_IP or SERVER_PORT constants and instead prompts for both values at startup. Document the interactive configuration so users do not edit nonexistent settings.

Before launching the client, open `run_client.py` and set the server address
and port to match the board configuration:

```python
SERVER_IP = "<board-ip>"
SERVER_PORT = 5000

**docs/installation.md:37**
* The address and port are adjusted interactively after launch, not by editing repository-defined defaults before launch. This closing instruction contradicts the actual startup flow in `run_client.py:33-46`.

The startup script currently points to the default server address and port defined in the repository. These values can be adjusted before launching the client if needed.

- **Files reviewed:** 12/13 changed files
- **Comments generated:** 0 new
- **Review effort level:** Balanced
</details>

[Copilot]

Copilot was unable to review this pull request because the user who requested the review is ineligible. To be eligible to request a review, you need a paid Copilot license, or your organization must enable Copilot code review.

[Copilot]

🔵 Needs a closer look

Several new instructions reference incorrect paths or misstate runtime networking and plotting requirements.

Review details

Suppressed comments (7)

Previously missed (4) — in code that hasn't changed since the last review.

docs/usage/experiment_definition.md:9

• This directory does not exist in this repository; the bundled experiment definitions are under yaml_experiment_configurations_examples/ (also documented in README.md:100). Naming the actual directory prevents users from looking for examples at a nonexistent path.

This issue also appears on line 212 of the same file.

Experiment definitions are stored in YAML format (typically within the `experiments/` directory) and structured into three primary top-level sections:

docs/index.md:41

• Contributors cannot update examples in experiments/ because this repository stores them in yaml_experiment_configurations_examples/ (README.md:100).

- adding or updating examples in `experiments/`;

docs/api.md:3

• The client uses a regular blocking socket (socket.create_connection in FIREQ_CLIENT/client.py:49); only the send and receive operations are moved to background worker threads. Calling the stream non-blocking misstates the networking model.

This issue also appears on line 11 of the same file.

Welcome to the `FIREQ-Client` API reference and architecture guide. The client acts as the front-end workstation interface that connects to the `FIREQ-Server`, processes experiment definitions, manages non-blocking TCP streams, and visualizes acquisition results.

docs/usage/plot.md:68

• plot_spectr rejects both inputs unless their acquisition $output_type is accumulated (FIREQ_PLOTTER/plotting/plot_spectr.py:40-43), but this requirement is omitted here. A user following the documented requirements with raw or decimated data will only receive an error.

- **Prerequisite:** Both target directories must contain `data.pkl` files from experiments executed with the exact same sweep vector (same range, step count, and variable target). Export swept experiments before plotting them.

docs/usage/experiment_definition.md:212

• The documented command points to a nonexistent experiments/ directory, so copying it from the guide fails to find an example. Use the repository's actual example directory.

run_yaml experiments/<experiment_name>.yaml

docs/api.md:11

• The socket is never configured as non-blocking; Client.connect() creates a normal blocking socket and delegates I/O to worker threads. Describe the connection layer without promising non-blocking socket sessions.

1. **Connection Layer**: Establishes and manages non-blocking TCP socket sessions with the server.

docs/installation.md:31

• This says the address and port must be adjusted before launch, but run_client.py:34-43 asks for both after launch. It also contradicts the startup-prompt instructions above and may lead users to edit the script unnecessarily.

The startup script currently points to the default server address and port defined in the repository. These values can be adjusted before launching the client if needed.

• Files reviewed: 12/13 changed files
• Comments generated: 0 new
• Review effort level: Balanced

[Copilot]

🔵 Needs a closer look

Several documented paths, sweep capabilities, and hardware configuration fields do not match the implementation.

Review details

Suppressed comments (13)

Previously missed (9) — in code that hasn't changed since the last review.

docs/usage/experiment_definition.md:9

• The repository’s example YAML files are in yaml_experiment_configurations_examples/, not experiments/ (as also documented in README.md:41). Pointing users to the current path avoids directing them to a directory that does not exist.

This issue also appears on line 64 of the same file.

Experiment definitions are stored in YAML format (typically within the `experiments/` directory) and structured into three primary top-level sections:

docs/usage/experiment_definition.md:212

• This command references an experiments/ directory that is absent from the repository, so it cannot run one of the supplied examples as written. Use the actual example directory shown in README.md:100.

run_yaml experiments/<experiment_name>.yaml

docs/index.md:41

• This contribution path does not exist; the repository stores its example configurations in yaml_experiment_configurations_examples/ (README.md:100). Contributors following this link would look in the wrong location.

- adding or updating examples in `experiments/`;

docs/usage/plot.md:21

• plot_2d cannot plot every non-swept experiment directly: for non-swept accumulated data it exits and instructs the user to use plot_iq; only raw/decimated non-swept data take the time-axis path. Qualify this statement so accumulated users are not sent to a command that produces no plot.

Visualizes standard $Y(X)$ response curves across parameter sweeps. The input
directory must contain `data.pkl`. Non-swept experiments can be plotted
directly after `run_yaml`; swept experiments require the client's `export`
command first.

docs/api.md:12

• Remove the duplicated space in the networking-layer description.

2. **Protocol & Networking Layer**: Implements length-prefixed binary framing  and manages background I/O threads.

docs/usage/experiment_definition.md:123

• These delay fields do not match the server constructor: it accepts _channel_mask (not _channel), requires _index, and does not accept _generate_trigger. A configuration following the current table can therefore fail with a missing or unexpected constructor argument.

| `_channel_mask` / `_channel` | `int` | Required | Channel index or bitmask target |
| `_index` | `int` | Optional | Sequence index for multi-drive delay tracking |
| `_generate_trigger` | `bool` | `False` | Fires a physical trigger upon delay counter expiration |

docs/usage/experiment_definition.md:37

• Top-level acquisition nodes are pre-created from the hardware tree and selected by the /axisAcquisitionIP_* path; _name is not a required YAML field, and adding it to that mapping is rejected as an unsupported scalar key. Mark it internal rather than required.

| `_name` | `str` | Yes | Name of the acquisition node instance |

docs/usage/experiment_definition.md:56

• As with acquisition nodes, generator nodes already exist and are addressed by their /axisGeneratorIP_* path. _name is constructor metadata, not a required field in the user’s YAML mapping.

| `_name` | `str` | Yes | Name of the signal generator instance |

docs/usage/experiment_definition.md:111

• Trigger generator nodes are also pre-existing hardware nodes selected by path, so _name must not be presented as a required YAML property; a scalar _name in this mapping is unsupported.

| `_name` | `str` | Yes | Name of the trigger generator instance |

docs/usage/experiment_definition.md:216

• The ordered list skips step 4, which makes the execution sequence appear incomplete.

5. wait for the acquisition to complete,
6. inspect the generated output files.

docs/usage/experiment_definition.md:64

• $tmanual_dest is an available generator parameter and determines whether a manual trigger targets the drive or readout wave. Omitting it from a table presented as the full parameter reference leaves the documented trigger_manually workflow under-specified.

| `$drive_order` | `list[str]` | No | Sequence list of pulse names to be executed |

docs/usage/experiment_definition.md:29

• The reference omits the root-level $shots parameter even though every run path indexes config["sys_config"]["$shots"] directly (FIREQ_CLIENT/client.py:215 and :298). Without documenting this required field, the advertised complete schema cannot be used to build a valid experiment.

This section details all available parameters, default behavior, and structural requirements for each node type in the `sys_config` hierarchy.

docs/usage/experiment_definition.md:12

• Logarithmic sweeps are not supported. The server recognizes lin, const, and list, but the client execution/export paths unconditionally require num and reconstruct values from start/stop, so only lin works end-to-end today.

2. **`variables`**: Specifies parameter sweeps using linear ranges.

• Files reviewed: 12/13 changed files
• Comments generated: 0 new
• Review effort level: Balanced

[Copilot]

🔵 Needs a closer look

The central documentation workflow cannot currently consume arbitrary documentation branches or generate the added autosummary API content, and several configuration fields are documented incorrectly.

Review details

Suppressed comments (9)

Previously missed (6) — in code that hasn't changed since the last review.

.github/workflows/trigger-fireq-docs.yml:6

• The downstream FIREQ-docs workflow ignores this dispatch's ref and sha; it checks out the client ref from fireq-release.yaml, currently docs/initial_doc. Consequently, pushes to any other matching docs/* branch trigger a rebuild of docs/initial_doc rather than the changed branch. Either keep this filter restricted to the pinned ref or update the downstream workflow to consume the dispatched ref/SHA.

      - "docs/**"

docs/api.md:36

• This API section cannot be generated by the current central documentation build: FIREQ-docs/docs/conf.py does not enable sphinx.ext.autosummary/autodoc, and its workflow does not install or add FIREQ-Client to Python's import path. These directives will therefore be unknown or unable to import their targets. Update the central builder accordingly, or replace this block with static reference content.

```{eval-rst}
.. rubric:: Core Client & CLI

.. autosummary::

docs/api.md:3

• The client does not use non-blocking sockets: socket.create_connection remains in blocking mode, while blocking reads and writes are delegated to worker threads. Calling these “non-blocking TCP streams” misstates the architecture.

Welcome to the `FIREQ-Client` API reference and architecture guide. The client acts as the front-end workstation interface that connects to the `FIREQ-Server`, processes experiment definitions, manages non-blocking TCP streams, and visualizes acquisition results.

docs/usage/experiment_definition.md:216

• The procedure jumps from step 3 to step 5, which makes the execution sequence incorrectly numbered.

5. wait for the acquisition to complete,
6. inspect the generated output files.

docs/usage/experiment_definition.md:37

• /axisAcquisitionIP_* identifies an already-created hardware node, so _name is not a YAML parameter and including it in this mapping would be rejected as an unsupported key. The complete example correctly omits it; mark it as internal rather than required for users.

This issue also appears in the following locations of the same file:

• line 56
• line 111

| `_name` | `str` | Yes | Name of the acquisition node instance |

docs/usage/experiment_definition.md:123

• These delay metadata rows do not match the server constructor: it accepts _channel_mask (not _channel), requires _index, and does not accept _generate_trigger. Following the current table with either alias or _generate_trigger causes a configuration error.

| `_channel_mask` / `_channel` | `int` | Required | Channel index or bitmask target |
| `_index` | `int` | Optional | Sequence index for multi-drive delay tracking |
| `_generate_trigger` | `bool` | `False` | Fires a physical trigger upon delay counter expiration |

docs/installation.md:31

• This contradicts the startup behavior described above: the address and port are entered at runtime, not adjusted in the script before launch.

The startup script currently points to the default server address and port defined in the repository. These values can be adjusted before launching the client if needed.

docs/usage/experiment_definition.md:56

• /axisGeneratorIP_* selects an existing server node, so users must not add _name to this YAML mapping; the server configuration walker only accepts callback keys and child definitions here. The example below correctly omits this field.

| `_name` | `str` | Yes | Name of the signal generator instance |

docs/usage/experiment_definition.md:111

• The trigger generator is also an existing hardware-tree node selected by its /axisTriggerGenerator_* path; _name is not a required user-supplied YAML field and would be rejected inside this mapping.

| `_name` | `str` | Yes | Name of the trigger generator instance |

• Files reviewed: 12/13 changed files
• Comments generated: 0 new
• Review effort level: Balanced

[Copilot]

Copilot was unable to review this pull request because the user who requested the review is ineligible. To be eligible to request a review, you need a paid Copilot license, or your organization must enable Copilot code review.