[rust] Add crates/platform-traits, the Rust half of the platform API - #746
Open
ganglyu wants to merge 2 commits into
Open
[rust] Add crates/platform-traits, the Rust half of the platform API#746ganglyu wants to merge 2 commits into
ganglyu wants to merge 2 commits into
Conversation
A plain library crate beside the Python `sonic_platform_base/`, holding the Rust side of the SONiC platform API. Nothing here reaches hardware: it is the contract a vendor implements and a daemon consumes, so porting a pmon daemon to Rust touches no vendor crate and asks no vendor for a second round of review. `PlatformApi` covers the whole platform API rather than only what the first consumer reads -- thermals, fans, fan drawers, PSUs, PDBs, LEDs and leak sensors -- with every accessor defaulting to "not supported", so a vendor implements what it has and says nothing about the rest. `ThermalManager` carries the start/stop hooks and the fan-speed policy, including the interval the policy runs on: Python takes that from the platform's thermal_policy.json rather than from the daemon's command line (thermal_manager_base.py:29, :155, :228), and 60 is what a file that omits the key gets. `run_policy` takes the platform, because Python passes the chassis and starts by collecting thermal information from it (:178-195). Types preserve distinctions the Python layer makes and downstream readers depend on. `Threshold` keeps int and float apart because `show platform temperature` reads the exact STATE_DB string, where Python prints "105" for an int and "105.0" for a float. `min_recorded`/`max_recorded` are `Option` because `ThermalBase` raises `NotImplementedError` there and thermalctld's `try_get` substitutes N/A; a platform that tracked the running extremes would print numbers where Python prints N/A. One `PsuInfo` carries both PSUs and PDBs, with the kind on each row telling them apart, because psud reads them through the same accessor names and publishes them under different key templates. Every optional field is `None` exactly where Python returns None, which is narrower than "the file could not be read". Shipping accessors ahead of a consumer means holding them to the same standard as the rest, so the crate carries its own tests: 9 covering the data types, the error kinds, and a bare-minimum implementation that exercises every default body -- which is how a vendor supports part of the API, and so is part of the contract rather than an implementation detail. Depends on nothing outside the standard library. `rust-version = "1.74"` is declared for `std::io::Error::other`, used by the `From<io::Error>` impl. .gitignore gains `target/` and `Cargo.lock`: the repository carries a Rust crate now and had no entries for one. The first consumer is the Rust `thermalctld` in sonic-platform-daemons. Signed-off-by: ganglyu <glv@nvidia.com>
Collaborator
|
/azp run |
|
Azure Pipelines: There may be pipelines that require an authorized user to comment /azp run to run. |
|
Azure Pipelines: Successfully started running 1 pipeline(s). |
rustfmt.toml, a Makefile and a .pre-commit-config.yaml, so the crate carries its own checks rather than relying on review to catch formatting. The formatting policy and the ci-* target names are sonic-dash-ha's, which is the only other Rust in SONiC, so a contributor moving between the two repositories runs the same commands and reads the same style. `make ci-all` is `cargo fmt --check`, clippy with clippy::all denied, build and doc with warnings denied, and the tests -- each of the last three in debug and release, because a lint or a cfg can differ between the two and what ships is release. Two deviations from sonic-dash-ha's Makefile, both deliberate. It runs `cargo clean` between the debug and release passes; that is a disk-space measure for its agent pool, and here it would throw away a contributor's build cache on every run, so it is omitted -- the two use separate target directories. And ci-doc passes --no-deps, so the verdict is about this repository's documentation rather than its dependencies'. The pre-commit hooks are scoped to Rust sources. The Python half of this repository predates them by years, and pointing a whitespace or end-of-file fixer at it would rewrite thousands of lines the first time anyone ran `pre-commit run --all-files`. Only the three hooks that cannot rewrite a file run repository-wide. The pinned revision is newer than sonic-dash-ha's v4.4.0, which emits a deprecation warning on every run. The rest of the diff is `cargo fmt` output. The crate was hand-formatted, so adopting the check required one mechanical pass; it changes no behaviour, and the tests pass unchanged. Two comments that read "Kept on one line" no longer described the code -- rustfmt breaks those calls regardless of max_width, because fn_call_width is 60% of it -- so they now record the tarpaulin coverage-attribution behaviour they were really about. Signed-off-by: ganglyu <glv@nvidia.com>
Collaborator
|
/azp run |
|
Azure Pipelines: Successfully started running 1 pipeline(s). |
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
2 participants
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.
Why I did it
SONiC pmon daemons are being ported to Rust one at a time. Each port needs a vendor-implemented platform API to read hardware through; without a shared contract, every port either names a vendor in the daemon or grows its own per-vendor trait.
This adds that contract. It lives in sonic-platform-common for the same reason
sonic_platform_base/does: it is the vendor-facing half, not the daemon-facing one.Work item tracking
How I did it
Added
crates/platform-traits, a plain library crate beside the Pythonsonic_platform_base/. Nothing in it reaches hardware — it is what a vendor implements and a daemon consumes. No Python file is touched and no existing behaviour changes.PlatformApicovers the whole platform API rather than only what the first consumer reads — thermals, fans, fan drawers, PSUs, PDBs, LEDs, leak sensors — with every accessor defaulting to "not supported", so a vendor implements what it has.ThermalManagercarries the start/stop hooks and the fan-speed policy, including the interval the policy runs on, which Python takes from the platform'sthermal_policy.json(thermal_manager_base.py:29, :155, :228).Thresholdkeepsintandfloatapart becauseshow platform temperaturereads the exact STATE_DB string;min_recorded/max_recordedareOptionbecauseThermalBaseraisesNotImplementedErrorthere andthermalctldsubstitutesN/A; onePsuInfocarries both PSUs and PDBs, aspsudreads them through the same accessors.std.rust-version = "1.74"forstd::io::Error::other..gitignoregainstarget/andCargo.lock.How to verify it
9 unit tests cover the data types, the error kinds, and a bare-minimum implementation that exercises every default body.
pytest tests/is unaffected.Which release branch to backport (provide reason below if selected)
N/A — new feature, no backport requested.
Tested branch
Description for the changelog
Add
crates/platform-traits, the vendor-agnostic Rust platform API traits for SONiC daemons.Link to config_db schema for YANG module changes
N/A — no CONFIG_DB schema change.