Repository navigation
Conversation
Python components need the same clock readings and quality rules as Rust callers. Expose the existing in-process implementation through optional PyO3 bindings, with read-only integer timestamps and source injection. Package the extension as a CPython 3.10+ ABI3 wheel and retain the upstream clock fixes and documentation. Validate the default and Python-enabled Rust clock tests, the installed wheel's five Python tests, uv lock consistency, and workspace format, clippy, build and tests. Assisted-by: Codex:gpt-6
enkerewpo
left a comment
There was a problem hiding this comment.
Ran head 4cfdfe0e locally (Linux x86_64, Python 3.12):
cargo test --locked -p robonix-chronos-clock --features python: 12 + 6 passedcargo clippy -p robonix-chronos-clock --features python --all-targets -- -D warnings: cleanuvx maturin build --release: builtrobonix_chronos_clock-1.1.0b1-cp310-abi3-manylinux_2_34_x86_64.whlpython -m unittest discover -s system/chronos/clock/tests -p test_python.py: 5/5 passeduv lock --check: passes
The bindings themselves look right. My questions are about how a component is supposed to use them (inline). One more thing for this PR: CI runs cargo clippy --workspace --tests and cargo test --workspace --all-targets without --features python, and nothing runs test_python.py, so none of the code added here is built or tested in CI. Please add a step that builds with --features python, installs the wheel and runs the Python tests.
| members = [ | ||
| "pylib/*", # shared Python libs (robonix-api, …) | ||
| "system/scene", # only Python-implemented system component today | ||
| "system/chronos/clock", # optional Python bindings for the Rust clock library |
There was a problem hiding this comment.
How will Scene and Soma depend on this? I added a throwaway workspace package with dependencies = ["robonix-api", "robonix-chronos-clock"] (both { workspace = true }) and ran uv sync. Without cargo on PATH it fails:
💥 maturin failed
Caused by: Cargo metadata failed. Does your crate compile with `cargo build`?
hint: `robonix-chronos-clock` was included because `clockuser` (v0.0.0) depends on `robonix-chronos-clock`
With cargo available it builds the crate in editable mode and the import works. So every environment that syncs a consumer package (package build scripts, the Scene image, Jetson boards) needs a Rust toolchain, or a prebuilt wheel for its platform. Please state which way it is meant to go, so the Scene and Soma follow-ups don't each solve it differently.
There was a problem hiding this comment.
Workspace development remains a source build: a consumer using the workspace dependency needs Rust and maturin when it syncs. Deployment uses a built wheel installed into the Python environment that actually runs the consumer. The build environment needs the toolchain; the runtime environment does not need Rust once the wheel is installed. We do not currently publish a platform wheel matrix.
The follow-up branches use that approach. Scene builds the wheel and installs it into its image or native Python environment. Local commit 9b938e0c makes the native build honor the same SCENE_NATIVE_PYTHON as startup, handles virtual environments without --user, and checks the import after installation and before startup. For Soma, local commit 07387184 adds an explicit installer accepting the Python executable used by runtime_reader_command; the reader reports that executable if the binding cannot load. Webots already installs the wheel while building the simulator image.
I tested wheel construction and installation on a Jetson running ARM64 Ubuntu 22.04 with glibc 2.35 and Python 3.10. The resulting manylinux_2_34_aarch64 wheel also installed and passed tests in the existing ARM Scene image. Native startup checks and a live Soma ROS reader test passed. Other deployments still need a wheel matching their architecture and Linux baseline; a host-built wheel is not guaranteed to work in an arbitrary container.
| time with the default quality limits and discards previous cached readings. | ||
| Call `clock.update_reference_time(timestamp_ns, generation, available, | ||
| uncertainty_ns=1, verified=False, reason="")` to install received readings. | ||
| The caller owns the ROS subscription and the supplied quality metadata. |
There was a problem hiding this comment.
Reference state is per process. In one process I called configure_reference("ros_time") and update_reference_time(42_000_000_000, 7, True); a child process started from it still read Linux time:
A ref 42000000000 gen 7
B ref 1793095309516759011 gen 1 available True
That matches the README, but it means every Python process that wants simulation time has to subscribe to /clock, track generations and call update_reference_time itself. Scene and Soma would both write that glue. Where should it live: a small ROS helper in this package, or robonix_api.ros? I would keep the native module out of robonix-api's hard dependencies, since that would put a Rust build in front of every package. An optional helper would still keep the logic in one place.
There was a problem hiding this comment.
The process-local behavior you observed is expected for this binding. The follow-up branches already encapsulate the simulation-time integration in component startup: Chronos owns the ROS /clock subscription and publishes reference readings, generations and quality through robonix/system/chronos/reference_time. Consumers receive that stream rather than subscribing to /clock themselves or generating their own simulation generations.
For Scene, scene_service/service.py enters the subscription context in scene_service/reference_time.py before running its active resources. That context selects the configured source, starts the background subscriber, forwards readings into update_reference_time(), handles reconnection and invalidation, and closes the subscription on exit. Scene's business code then reads the local clock interface; it does not establish a connection on each read. Selecting simulation time makes reference reads unavailable until input arrives, rather than continuing with the default Linux reference.
Soma has the equivalent startup integration in Rust through ReferenceSubscription; its Python ROS reader only uses wall_timestamp_ns() for receipt timestamps. It does not need a second Python simulation-time subscriber.
Each consumer process still owns a connection to the Chronos reference stream and its own local cache. Those connections are managed by the existing lifecycle integrations in the follow-ups. Importing the native binding alone does not initialize a new process from deployment configuration, and there is not yet one generic SDK initializer shared by all Python consumers. This PR keeps the binding independent of ROS, Atlas and gRPC, and does not make it a mandatory dependency of robonix-api. The existing startup integrations provide the current consumers' initialization without an additional time-reading API; sharing their client transport code can be discussed separately.
|
|
||
| [project] | ||
| name = "robonix-chronos-clock" | ||
| version = "1.1.0b1" |
There was a problem hiding this comment.
This duplicates the workspace version (1.1.0-beta.1 in Cargo.toml) by hand. maturin can take it from Cargo with dynamic = ["version"], which avoids the two drifting at the next release.
There was a problem hiding this comment.
I replaced the static Python version with dynamic = ["version"] in commit 6e6a983c, so maturin takes it from the Cargo workspace. I also regenerated the uv lock metadata. Wheels built and installed on x86_64 and ARM64 report 1.1.0b1, the Python-normalized form of Cargo's 1.1.0-beta.1. The lockfile checks passed.
Default workspace checks leave the optional Python bindings disabled. Add feature-enabled Rust checks, then build and install the wheel before running the Python tests. Feature-enabled Rust tests, Clippy and installed-wheel Python tests passed on x86_64 and ARM64 Linux. Assisted-by: Codex:gpt-6.1-terra Assisted-by: Codex:gpt-6.1-sol
The Python package repeats the Cargo version, which can drift at release time. Let maturin read it from Cargo and refresh the uv lock metadata. Built and installed wheels on x86_64 and ARM64 Linux; both report the expected Python version 1.1.0b1. Lockfile checks passed. Assisted-by: Codex:gpt-6.1-terra Assisted-by: Codex:gpt-6.1-sol
Python components need the same clock behavior as Rust callers for reference-time freshness checks, simulation pauses and resets, and explicit clock errors. This PR exposes the existing Chronos implementation through optional PyO3 bindings and adds the robonix-chronos-clock Python package.
The API provides reference, steady and wall time, paired snapshots, source selection and injected reference readings. Returned fields are read-only and retain integer nanosecond precision, quality metadata and generation information. Clock-read failures raise OSError. The implementation preserves the upstream clock fixes.
Later changes will use this package in:
Validation:
The wheel targets CPython 3.10+ and must be built separately for each platform and architecture.