Add linux support by felipebalbi · Pull Request #40 · OpenDevicePartnership/odp-platform-common

felipebalbi · 2026-04-14T22:16:53Z

WIP: Linux support

Copilot

Pull request overview

This PR is a WIP toward Linux support by introducing an OS-backed data source (OsSource) in ec-test-lib and switching the CLI/TUI to use runtime source selection rather than transport feature flags.

Changes:

Add ec-test-lib::os::OsSource with per-OS implementations (Windows ACPI-backed, Linux sysfs/ioctl-backed) and re-export via ec_test_lib::os.
Update ec-test-cli and ec-test-tui to instantiate OsSource for the “Acpi” source kind, and remove acpi/transport feature sections from the binaries’ Cargo.toml.
Add Linux-only dependency (libc) and rework Windows dependencies to be target-specific.

Reviewed changes

Copilot reviewed 10 out of 11 changed files in this pull request and generated 8 comments.

Show a summary per file

File	Description
ec/test-tui/src/main.rs	Switches the “Acpi” branch to use `ec_test_lib::os::OsSource`; removes `cfg(feature="acpi")` gating.
ec/test-tui/Cargo.toml	Removes `acpi` feature section.
ec/test-lib/src/os/windows.rs	Renames the Windows ACPI-backed source type to `OsSource` and updates internal references.
ec/test-lib/src/os/mod.rs	Adds OS-dispatch module (`windows.rs` vs `linux.rs`) and re-exports implementation.
ec/test-lib/src/os/linux.rs	Adds a Linux `OsSource` implementation using sysfs for thermal/battery and ioctl for RTC.
ec/test-lib/src/lib.rs	Exposes `pub mod os;` and removes the old feature-gated ACPI module export/guard.
ec/test-lib/Cargo.toml	Moves Windows deps under target cfg, adds Linux `libc`, and removes the `acpi` feature definition.
ec/test-cli/src/main.rs	Switches to runtime `--source` dispatch and uses `os::OsSource` for “Acpi”.
ec/test-cli/src/cli.rs	Adds `--source` and updates serial args to be conditionally required.
ec/test-cli/Cargo.toml	Removes transport feature flags.
ec/Cargo.lock	Adds `libc` to the lockfile due to Linux support.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-04-14T22:21:17Z

@@ -27,8 +27,5 @@ critical-section.workspace = true
 # EC transport library
 ec-test-lib = { path = "../test-lib" }



ec-test-tui no longer defines the transport feature flags, but the repo CI uses cargo-hack with --mutually-exclusive-features mock,serial and --exclude-features acpi for this crate. Either reintroduce those (even as empty flags) or update the CI workflow to reflect runtime selection, otherwise CI will fail.

Suggested change

[features]

default = []

mock = []

serial = []

acpi = []

Copilot · 2026-04-14T22:21:17Z

+    Mock,
+    /// Real hardware via serial transport.
+    Serial,
+    /// Real hardware via ACPI (Windows only).


The help text says this source is "ACPI (Windows only)", but the code now routes this variant to ec_test_lib::os::OsSource, which also has a Linux implementation. Either rename this variant (e.g. Os/System) or update the doc comment/value names so the CLI accurately reflects what --source acpi does on Linux vs Windows.

Suggested change

/// Real hardware via ACPI (Windows only).

/// Real hardware via the OS-provided hardware interface

/// (ACPI on Windows; also supported on Linux).

Copilot · 2026-04-14T22:21:18Z

 /// Available data sources (only variants whose feature is compiled in are shown).
 #[derive(clap::ValueEnum, Clone, Copy)]
 enum SourceKind {
    /// Deterministic in-process mock — no hardware required.
    Mock,
    /// Real hardware via serial transport.
    Serial,
    /// Real hardware via ACPI (aarch64-pc-windows-msvc only).
-    #[cfg(feature = "acpi")]
    Acpi,
 }


These comments still refer to feature-gated sources ("only variants whose feature is compiled in" / "ACPI ... aarch64-pc-windows-msvc only"), but the Acpi variant is now always present and is backed by ec_test_lib::os::OsSource (which also has a Linux implementation). Please update the enum/docs so the runtime options and platform support are described accurately.

Copilot · 2026-04-14T22:21:18Z

+    fn get_temperature(&self) -> Result<f64, Self::Error> {
+        let zone = thermal_zone_path()?;
+        let millideg = read_u64(&zone.join("temp"))? as i64;
+        Ok(millideg as f64 / 1000.0)
+    }
+
+    fn get_rpm(&self) -> Result<f64, Self::Error> {
+        Ok(read_u64(&hwmon_path()?.join("fan1_input"))? as f64)
+    }


thermal_zone_path() scans /sys/class/thermal and reads multiple files each call, and hwmon_path() scans /sys/class/hwmon. Because the TUI polls these methods every second (and calls thresholds multiple times per update), this can generate a lot of filesystem churn. Consider caching discovered sysfs paths (battery/hwmon/thermal zone) inside OsSource during construction so repeated reads only hit the specific files.

Copilot · 2026-04-14T22:21:18Z

+fn open_rtc() -> Result<OwnedFd, Error> {
+    let path = c"/dev/rtc0";
+    let fd = unsafe { libc::open(path.as_ptr(), libc::O_RDONLY) };
+    if fd < 0 {
+        Err(Error::Io(io::Error::last_os_error()))
+    } else {
+        Ok(OwnedFd(fd))
+    }


open_rtc() uses libc::open without O_CLOEXEC, which can leak the RTC fd across exec in long-running processes. Prefer opening /dev/rtc0 via std::fs::File::open (and using AsRawFd) or pass O_CLOEXEC to open so the fd is not inherited.

Copilot · 2026-04-14T22:21:18Z

+#[cfg_attr(target_os = "windows", path = "windows.rs")]
+#[cfg_attr(target_os = "linux", path = "linux.rs")]
+mod imp;
+
+pub use imp::*;


os::mod only selects windows.rs/linux.rs via cfg_attr. On any other target OS (e.g., macOS), the module will fall back to looking for os/imp.rs and fail to compile. Add an explicit #[cfg(not(any(target_os = "windows", target_os = "linux")))] compile_error!(...) or provide a stub imp implementation for unsupported targets so the failure mode is clear and intentional.

Copilot · 2026-04-14T22:21:18Z

+[target.'cfg(target_os = "linux")'.dependencies]
+libc = { version = "0.2" }

 [features]


This crate no longer defines the acpi feature, but the repository CI/workflows (e.g. cargo hack --exclude-features acpi / cargo xwin ... --features acpi) still assume it exists. Either restore an acpi feature flag (even as a no-op/alias) or update the CI configuration accordingly, otherwise CI will fail.

Suggested change

[features]

[features]

acpi = []

Copilot · 2026-04-14T22:21:19Z

@@ -14,10 +14,5 @@ ec-test-lib = { path = "../test-lib" }
 time-alarm-service-messages.workspace = true
 battery-service-messages.workspace = true



ec-test-cli no longer defines mock/serial/acpi features, but the repo CI uses cargo-hack with --mutually-exclusive-features mock,serial and --exclude-features acpi for this crate. Either keep these features (even as empty feature flags) or update the CI workflow; otherwise CI will error when it tries to reason about missing features.

Suggested change

[features]

mock = []

serial = []

acpi = []

felipebalbi added 2 commits April 14, 2026 13:28

Completely remove acpi feature

b082614

Add linux support

7c90454

Copilot AI review requested due to automatic review settings April 14, 2026 22:16

Copilot started reviewing on behalf of felipebalbi April 14, 2026 22:17 View session

Copilot AI reviewed Apr 14, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add linux support#40

Add linux support#40
felipebalbi wants to merge 2 commits into
OpenDevicePartnership:mainfrom
felipebalbi:add-linux-support

felipebalbi commented Apr 14, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Apr 14, 2026

Uh oh!

Copilot AI Apr 14, 2026

Uh oh!

Copilot AI Apr 14, 2026

Uh oh!

Copilot AI Apr 14, 2026

Uh oh!

Copilot AI Apr 14, 2026

Uh oh!

Copilot AI Apr 14, 2026

Uh oh!

Copilot AI Apr 14, 2026

Uh oh!

Copilot AI Apr 14, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

		@@ -27,8 +27,5 @@ critical-section.workspace = true
		# EC transport library
		ec-test-lib = { path = "../test-lib" }

+[features]
+default = []
+mock = []
+serial = []
+acpi = []

	/// Real hardware via ACPI (Windows only).
	/// Real hardware via the OS-provided hardware interface
	/// (ACPI on Windows; also supported on Linux).

		@@ -14,10 +14,5 @@ ec-test-lib = { path = "../test-lib" }
		time-alarm-service-messages.workspace = true
		battery-service-messages.workspace = true

Conversation

felipebalbi commented Apr 14, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Apr 14, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 14, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 14, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 14, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 14, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 14, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 14, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 14, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants