ONVIF camera control + cargo-dist release pipeline#10
Merged
Conversation
Cameras with `media_sources[].control:` now register as Devices alongside their streaming path, so `osdl send cam1 ptz_move|ptz_stop|ptz_preset_goto| snapshot` flows through the existing send_command pipeline. No new RPCs, no camera-specific subcommand. Streaming behavior is unchanged for cameras without a `control` block. - adapter/onvif.rs: built-in adapter (no YAML registry); JSON-envelope encoding so the byte-oriented Transport trait stays uniform; direction shorthand (up/down/left/right/zoom_in/zoom_out) plus explicit pan/tilt/ zoom vectors. - transport/onvif.rs: HTTP/SOAP client with WS-Security UsernameToken digest auth; quick-xml parsing of GetCapabilities/GetProfiles/ GetSnapshotUri; per-camera auto-stop tracker so back-to-back ptz_move calls or an explicit ptz_stop cancel a prior in-flight stop task. - engine: walks media_sources after mediamtx spawns and dual-registers cameras with `control:` set. Idempotent across mediamtx restarts. - config: OsdlConfig.data_dir threaded through; snapshots land under <data_dir>/snapshots/<cam_id>/<ts>.jpg with path/url surfaced as device_status properties. Verified live against two JZT31 cameras: PTZ in all directions, auto-stop cancellation across overlapping moves, and a 372 KB snapshot saved to disk and reopened cleanly. Co-Authored-By: Claude Opus 4.7 <[email protected]>
`dist init` configures GitHub Actions to build pre-built binaries for every tagged `v*` push. Targets cover Apple Silicon + Intel macOS, glibc and musl Linux on x86_64 + aarch64, and x86_64 Windows. Floor glibc 2.17 (RHEL 7+, Ubuntu 14.04+) via cargo-zigbuild + the `min-glibc-version` knob. Shell + PowerShell installers are generated so users get the standard `curl … | sh` / `irm … | iex` install path. Also propagates `repository = …` to every workspace member via `workspace.package` so cargo-dist's GitHub-CI integration knows where to publish. README gains an Install section pointing at the installer scripts. Daily development is unaffected — the workflow only publishes when a SemVer tag is pushed; PRs trigger a validation-only `dist plan` run. Co-Authored-By: Claude Opus 4.7 <[email protected]>
The file was a one-off snapshot of in-flight firmware work that was superseded by the per-MCU layout in firmware/ (commit e2304a1) and the per-station notes living next to each binary's source. Keeping it around just rots — the live `cargo build` matrix is the source of truth now. Co-Authored-By: Claude Opus 4.7 <[email protected]>
cargo-dist defaults to the runner's glibc (2.31+ on ubuntu-22.04), which silently shuts RHEL 7 / CentOS 7 / Ubuntu 14.04-18.04 out of running our binaries. 2.17 is the lowest practical floor — same target Astral's uv pins, same one we've already verified locally with `cargo zigbuild --target=...gnu.2.17`. aarch64 needs 2.28 because no real aarch64 distro ever shipped older. Co-Authored-By: Claude Opus 4.7 <[email protected]>
Reviewer's Guide将 ONVIF 摄像机控制作为一等公民的设备/传输方式（PTZ 和快照），并将其接入现有的 通过 send_command 进行 ONVIF 摄像机控制的时序图sequenceDiagram
actor User
participant OsdlCli as osdl_cli
participant Engine as OsdlEngine
participant Adapter as OnvifAdapter
participant Transport as OnvifTransport
participant Cam as OnvifCamera
User->>OsdlCli: osdl send cam1 ptz_move -p direction=up
OsdlCli->>Engine: send_command(device_id=cam1, action=ptz_move)
Engine->>Adapter: encode_command(device_type=onvif_camera_ptz, cmd)
Adapter-->>Engine: JSON envelope { op: "ptz_move", args: ... }
Engine->>Transport: send(bytes=envelope)
Transport->>Transport: dispatch("ptz_move", args)
Transport->>Cam: SOAP ContinuousMove + scheduled Stop
Transport-->>Engine: TransportRx{ transport_id, data=JSON result }
Engine->>Adapter: decode_response(device_type, bytes)
Adapter-->>Engine: properties (e.g. ptz_move_ok)
Engine-->>OsdlCli: DeviceStatus event
User->>OsdlCli: osdl send cam1 snapshot
OsdlCli->>Engine: send_command(device_id=cam1, action=snapshot)
Engine->>Adapter: encode_command(... snapshot ...)
Adapter-->>Engine: JSON envelope { op: "snapshot" }
Engine->>Transport: send(bytes=envelope)
Transport->>Cam: SOAP GetSnapshotUri + HTTP GET JPEG
Transport->>Transport: write JPEG to <data_dir>/snapshots/cam1/<ts>.jpg
Transport-->>Engine: TransportRx{ data: { op:"snapshot", ok:true, data:{ path, url } } }
Engine->>Adapter: decode_response(...)
Adapter-->>Engine: properties (snapshot_path, snapshot_url)
Engine-->>OsdlCli: DeviceStatus with snapshot info
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your Experience访问你的 dashboard 以：
Getting HelpOriginal review guide in EnglishReviewer's GuideAdds ONVIF camera control as a first-class device/transport (PTZ and snapshots) and wires it into the existing send_command pipeline, introduces a configurable data_dir for snapshot storage, updates the CLI and example configs to use the new path and adapter, and sets up a cargo-dist based GitHub Actions release pipeline with prebuilt binaries and installers while cleaning up obsolete firmware docs and workspace metadata. Sequence diagram for ONVIF camera control via send_commandsequenceDiagram
actor User
participant OsdlCli as osdl_cli
participant Engine as OsdlEngine
participant Adapter as OnvifAdapter
participant Transport as OnvifTransport
participant Cam as OnvifCamera
User->>OsdlCli: osdl send cam1 ptz_move -p direction=up
OsdlCli->>Engine: send_command(device_id=cam1, action=ptz_move)
Engine->>Adapter: encode_command(device_type=onvif_camera_ptz, cmd)
Adapter-->>Engine: JSON envelope { op: "ptz_move", args: ... }
Engine->>Transport: send(bytes=envelope)
Transport->>Transport: dispatch("ptz_move", args)
Transport->>Cam: SOAP ContinuousMove + scheduled Stop
Transport-->>Engine: TransportRx{ transport_id, data=JSON result }
Engine->>Adapter: decode_response(device_type, bytes)
Adapter-->>Engine: properties (e.g. ptz_move_ok)
Engine-->>OsdlCli: DeviceStatus event
User->>OsdlCli: osdl send cam1 snapshot
OsdlCli->>Engine: send_command(device_id=cam1, action=snapshot)
Engine->>Adapter: encode_command(... snapshot ...)
Adapter-->>Engine: JSON envelope { op: "snapshot" }
Engine->>Transport: send(bytes=envelope)
Transport->>Cam: SOAP GetSnapshotUri + HTTP GET JPEG
Transport->>Transport: write JPEG to <data_dir>/snapshots/cam1/<ts>.jpg
Transport-->>Engine: TransportRx{ data: { op:"snapshot", ok:true, data:{ path, url } } }
Engine->>Adapter: decode_response(...)
Adapter-->>Engine: properties (snapshot_path, snapshot_url)
Engine-->>OsdlCli: DeviceStatus with snapshot info
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - 我发现了 1 个问题，并留下了一些总体层面的反馈：
- ONVIF 传输层在异步方法（例如
do_snapshot）中执行了同步的文件系统操作（create_dir_all、write）；建议将这些操作切换到tokio::fs或使用spawn_blocking，以避免在大文件或慢写入时阻塞异步运行时。 - 用于 WS-Security 时间戳的自定义
epoch_to_ymdhms/format_created_now日历计算逻辑比较复杂；使用经过充分测试的时间/日期库（例如time或chrono）可以降低出现细微 Bug 的风险，并简化实现。
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The ONVIF transport does synchronous filesystem work (`create_dir_all`, `write`) inside async methods like `do_snapshot`; consider switching to `tokio::fs` or `spawn_blocking` for those operations to avoid blocking the async runtime on large/slow writes.
- The custom `epoch_to_ymdhms`/`format_created_now` calendar math for WS-Security timestamps is non-trivial; using a well‑tested time/date crate (e.g. `time` or `chrono`) would reduce the risk of subtle bugs and simplify the implementation.
## Individual Comments
### Comment 1
<location path="crates/osdl-core/src/transport/onvif.rs" line_range="331-340" />
<code_context>
+ } else {
+ req
+ };
+ let resp = req.send().await.map_err(|e| format!("snapshot GET: {e}"))?;
+ let status = resp.status();
+ let bytes = resp
+ .bytes()
+ .await
+ .map_err(|e| format!("snapshot read: {e}"))?;
+ if !status.is_success() {
+ return Err(format!("snapshot HTTP {status}"));
+ }
+
+ let now_ms = SystemTime::now()
+ .duration_since(UNIX_EPOCH)
+ .map(|d| d.as_millis())
+ .unwrap_or(0);
+ let cam_dir = self.inner.snapshot_root.join(&self.inner.camera_id);
+ std::fs::create_dir_all(&cam_dir)
+ .map_err(|e| format!("create snapshot dir: {e}"))?;
+ let path = cam_dir.join(format!("{now_ms}.jpg"));
+ std::fs::write(&path, &bytes)
+ .map_err(|e| format!("write snapshot: {e}"))?;
+
</code_context>
<issue_to_address>
**suggestion (performance):** Blocking filesystem operations (`create_dir_all`/`write`) inside async code can stall the runtime under load.
These `std::fs::create_dir_all` and `std::fs::write` calls run directly in the async `do_snapshot` path and can block a Tokio worker thread under load, impacting other tasks. Prefer Tokio’s async filesystem APIs:
```rust
use tokio::fs;
fs::create_dir_all(&cam_dir)
.await
.map_err(|e| format!("create snapshot dir: {e}"))?;
fs::write(&path, &bytes)
.await
.map_err(|e| format!("write snapshot: {e}"))?;
```
If you must keep `std::fs`, consider wrapping these in `tokio::task::spawn_blocking` instead.
Suggested implementation:
```rust
use serde_json::{json, Value};
use sha1::{Digest, Sha1};
use std::path::PathBuf;
use std::sync::Arc;
use std::time::{Duration, SystemTime, UNIX_EPOCH};
use tokio::fs;
```
```rust
let now_ms = SystemTime::now()
.duration_since(UNIX_EPOCH)
.map(|d| d.as_millis())
.unwrap_or(0);
let cam_dir = self.inner.snapshot_root.join(&self.inner.camera_id);
fs::create_dir_all(&cam_dir)
.await
.map_err(|e| format!("create snapshot dir: {e}"))?;
let path = cam_dir.join(format!("{now_ms}.jpg"));
fs::write(&path, &bytes)
.await
.map_err(|e| format!("write snapshot: {e}"))?;
```
</issue_to_address>帮我变得更有用！请在每条评论上点 👍 或 👎，我会根据你的反馈改进后续评审。
Original comment in English
Hey - I've found 1 issue, and left some high level feedback:
- The ONVIF transport does synchronous filesystem work (
create_dir_all,write) inside async methods likedo_snapshot; consider switching totokio::fsorspawn_blockingfor those operations to avoid blocking the async runtime on large/slow writes. - The custom
epoch_to_ymdhms/format_created_nowcalendar math for WS-Security timestamps is non-trivial; using a well‑tested time/date crate (e.g.timeorchrono) would reduce the risk of subtle bugs and simplify the implementation.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The ONVIF transport does synchronous filesystem work (`create_dir_all`, `write`) inside async methods like `do_snapshot`; consider switching to `tokio::fs` or `spawn_blocking` for those operations to avoid blocking the async runtime on large/slow writes.
- The custom `epoch_to_ymdhms`/`format_created_now` calendar math for WS-Security timestamps is non-trivial; using a well‑tested time/date crate (e.g. `time` or `chrono`) would reduce the risk of subtle bugs and simplify the implementation.
## Individual Comments
### Comment 1
<location path="crates/osdl-core/src/transport/onvif.rs" line_range="331-340" />
<code_context>
+ } else {
+ req
+ };
+ let resp = req.send().await.map_err(|e| format!("snapshot GET: {e}"))?;
+ let status = resp.status();
+ let bytes = resp
+ .bytes()
+ .await
+ .map_err(|e| format!("snapshot read: {e}"))?;
+ if !status.is_success() {
+ return Err(format!("snapshot HTTP {status}"));
+ }
+
+ let now_ms = SystemTime::now()
+ .duration_since(UNIX_EPOCH)
+ .map(|d| d.as_millis())
+ .unwrap_or(0);
+ let cam_dir = self.inner.snapshot_root.join(&self.inner.camera_id);
+ std::fs::create_dir_all(&cam_dir)
+ .map_err(|e| format!("create snapshot dir: {e}"))?;
+ let path = cam_dir.join(format!("{now_ms}.jpg"));
+ std::fs::write(&path, &bytes)
+ .map_err(|e| format!("write snapshot: {e}"))?;
+
</code_context>
<issue_to_address>
**suggestion (performance):** Blocking filesystem operations (`create_dir_all`/`write`) inside async code can stall the runtime under load.
These `std::fs::create_dir_all` and `std::fs::write` calls run directly in the async `do_snapshot` path and can block a Tokio worker thread under load, impacting other tasks. Prefer Tokio’s async filesystem APIs:
```rust
use tokio::fs;
fs::create_dir_all(&cam_dir)
.await
.map_err(|e| format!("create snapshot dir: {e}"))?;
fs::write(&path, &bytes)
.await
.map_err(|e| format!("write snapshot: {e}"))?;
```
If you must keep `std::fs`, consider wrapping these in `tokio::task::spawn_blocking` instead.
Suggested implementation:
```rust
use serde_json::{json, Value};
use sha1::{Digest, Sha1};
use std::path::PathBuf;
use std::sync::Arc;
use std::time::{Duration, SystemTime, UNIX_EPOCH};
use tokio::fs;
```
```rust
let now_ms = SystemTime::now()
.duration_since(UNIX_EPOCH)
.map(|d| d.as_millis())
.unwrap_or(0);
let cam_dir = self.inner.snapshot_root.join(&self.inner.camera_id);
fs::create_dir_all(&cam_dir)
.await
.map_err(|e| format!("create snapshot dir: {e}"))?;
let path = cam_dir.join(format!("{now_ms}.jpg"));
fs::write(&path, &bytes)
.await
.map_err(|e| format!("write snapshot: {e}"))?;
```
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Comment on lines
+331
to
+340
| let resp = req.send().await.map_err(|e| format!("snapshot GET: {e}"))?; | ||
| let status = resp.status(); | ||
| let bytes = resp | ||
| .bytes() | ||
| .await | ||
| .map_err(|e| format!("snapshot read: {e}"))?; | ||
| if !status.is_success() { | ||
| return Err(format!("snapshot HTTP {status}")); | ||
| } | ||
|
|
There was a problem hiding this comment.
suggestion (performance): 在异步代码中执行阻塞的文件系统操作（create_dir_all/write）会在高负载下拖慢运行时。
这些 std::fs::create_dir_all 和 std::fs::write 调用直接运行在异步的 do_snapshot 调用路径上，在高负载下可能阻塞 Tokio 的工作线程，影响其他任务。建议优先使用 Tokio 的异步文件系统 API：
use tokio::fs;
fs::create_dir_all(&cam_dir)
.await
.map_err(|e| format!("create snapshot dir: {e}"))?;
fs::write(&path, &bytes)
.await
.map_err(|e| format!("write snapshot: {e}"))?;如果必须保留 std::fs，可以考虑把这些操作包在 tokio::task::spawn_blocking 中。
建议的实现：
use serde_json::{json, Value};
use sha1::{Digest, Sha1};
use std::path::PathBuf;
use std::sync::Arc;
use std::time::{Duration, SystemTime, UNIX_EPOCH};
use tokio::fs; let now_ms = SystemTime::now()
.duration_since(UNIX_EPOCH)
.map(|d| d.as_millis())
.unwrap_or(0);
let cam_dir = self.inner.snapshot_root.join(&self.inner.camera_id);
fs::create_dir_all(&cam_dir)
.await
.map_err(|e| format!("create snapshot dir: {e}"))?;
let path = cam_dir.join(format!("{now_ms}.jpg"));
fs::write(&path, &bytes)
.await
.map_err(|e| format!("write snapshot: {e}"))?;Original comment in English
suggestion (performance): Blocking filesystem operations (create_dir_all/write) inside async code can stall the runtime under load.
These std::fs::create_dir_all and std::fs::write calls run directly in the async do_snapshot path and can block a Tokio worker thread under load, impacting other tasks. Prefer Tokio’s async filesystem APIs:
use tokio::fs;
fs::create_dir_all(&cam_dir)
.await
.map_err(|e| format!("create snapshot dir: {e}"))?;
fs::write(&path, &bytes)
.await
.map_err(|e| format!("write snapshot: {e}"))?;If you must keep std::fs, consider wrapping these in tokio::task::spawn_blocking instead.
Suggested implementation:
use serde_json::{json, Value};
use sha1::{Digest, Sha1};
use std::path::PathBuf;
use std::sync::Arc;
use std::time::{Duration, SystemTime, UNIX_EPOCH};
use tokio::fs; let now_ms = SystemTime::now()
.duration_since(UNIX_EPOCH)
.map(|d| d.as_millis())
.unwrap_or(0);
let cam_dir = self.inner.snapshot_root.join(&self.inner.camera_id);
fs::create_dir_all(&cam_dir)
.await
.map_err(|e| format!("create snapshot dir: {e}"))?;
let path = cam_dir.join(format!("{now_ms}.jpg"));
fs::write(&path, &bytes)
.await
.map_err(|e| format!("write snapshot: {e}"))?;
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Two threads, separate concerns, but small enough to land together:
Camera control plane (
d622ef5)ONVIF cameras with a
control:block inmedia_sourcesnow register as Devices alongside their existing streaming path, soosdl send cam1 ptz_move|ptz_stop|ptz_preset_goto|snapshotflows through the standardsend_commandpipeline. No new RPCs, no new CLI subcommand. Streaming behavior is unchanged for cameras without acontrol:block — fully backward compatible.adapter/onvif.rs— built-in adapter (no YAML registry); JSON-envelope encoding so the byte-orientedTransporttrait stays uniform; direction shorthand (up/down/left/right/zoom_in/zoom_out) plus explicit pan/tilt/zoom vectors.transport/onvif.rs— HTTP/SOAP client with WS-Security UsernameToken digest auth;quick-xmlparsing of GetCapabilities/GetProfiles/GetSnapshotUri (replaces an earlier hand-rolled XML walker that was vulnerable to lookalike tags); per-camera auto-stop tracker so back-to-backptz_movecalls or an explicitptz_stopcancel a prior in-flight stop task.engine.rs— walksmedia_sourcesafter mediamtx spawns and dual-registers cameras withcontrol:set. Idempotent across mediamtx restarts.config.rs— addsOsdlConfig.data_dirso snapshots have a stable home (<data_dir>/snapshots/<cam_id>/<ts>.jpg); path/URL surface asdevice_statusproperties.Release pipeline via cargo-dist (
5c2a199+fba14a7)dist initconfigures GitHub Actions to build pre-built binaries for every taggedv*push. Daily development is unaffected — PRs trigger a 30s validation-onlydist planrun; releases only fire on tag push.2.17on x86_64 (RHEL 7+, Ubuntu 14.04+),2.28on aarch64 (no real aarch64 distro shipped older). Same target uv pins. Without this, cargo-dist would silently link against ubuntu-22.04's GLIBC_2.31 and shut RHEL 7 / CentOS 7 users out.installer.sh(curl-pipe-sh) andinstaller.ps1(irm-iex). Auto-detect OS/arch/libc and drop into\$CARGO_HOME/bin. README updated with one-liner install commands.osdl-updatecompanion binary so users can self-upgrade without checking releases manually.repository = …to every workspace member viaworkspace.packageso the GitHub-CI integration knows where to publish.Housekeeping (
978738d)Remove
firmware_status.md— was a one-off bring-up snapshot superseded by per-MCU firmware crate layout (#7). Livecargo buildmatrix is the source of truth now.Test plan
cargo test --workspace --features espnow— 161 tests pass (unit + integration + e2e)ptz_movecallscargo zigbuild --target x86_64-unknown-linux-gnu.2.17 --features espnow --bin osdl— confirms glibc 2.17 binary builds with the new ONVIF deps (verified via objdump symbol-version walk)dist plansucceeds with the configured targets + glibc pinv0.1.0) — deferred until this lands; will validate the actual workflow runOut of scope
snapshot_urlis currentlyfile://only. Mediamtx already runs an HTTP listener on:8888; teaching it to serve<data_dir>/snapshots/would let us return a real URL for cloud-WS consumers. Tracked in the Xyzen-side integration doc (Xyzen/osdl-camera-control.md).tr2:Media2) — Media v1 only for now.installersuntil someone asks; it's a one-line config flip.🤖 Generated with Claude Code
Sourcery 总结
在引擎中将 ONVIF 摄像机控制作为一等设备支持，并集成基于 cargo-dist 的发布管线，包括安装脚本和元数据更新。
新功能：
增强：
构建：
CI：
文档：
测试：
杂务：
Original summary in English
Summary by Sourcery
Add ONVIF camera control as a first-class device in the engine and integrate a cargo-dist based release pipeline with install scripts and metadata updates.
New Features:
Enhancements:
Build:
CI:
Documentation:
Tests:
Chores: