From baada36222f46c5e9c7d129b90f487bdd5219bd5 Mon Sep 17 00:00:00 2001
From: nevaforget <dom@moonarch.de>
Date: Wed, 10 Jun 2026 17:51:40 +0200
Subject: [PATCH] fix: reject out-of-range led brightness instead of silently
 clamping

led accepted any u16 and main.rs clamped to 0..=1000 with no feedback,
inconsistent with sidetone which rejects out-of-range at parse time.
Add a clap range validator (0..=1000) and drop the silent clamp, so
invalid input now fails loudly.

Further quality-audit follow-ups:
- remove dead BragiDevice::open() (no caller; binary uses open_with_verbose)
- add tests: led range validation, format_battery for all status
  variants, waybar "unknown" class

Bump 0.1.2 -> 0.1.3.
---
 Cargo.lock           |  2 +-
 Cargo.toml           |  2 +-
 src/bragi/device.rs  |  6 +-----
 src/cli.rs           |  1 +
 src/main.rs          |  5 ++---
 tests/cli_test.rs    | 12 ++++++++++++
 tests/output_test.rs | 31 +++++++++++++++++++++++++++++++
 7 files changed, 49 insertions(+), 10 deletions(-)

diff --git a/Cargo.lock b/Cargo.lock
index 2375eb9..228040b 100644
--- a/Cargo.lock
+++ b/Cargo.lock
@@ -144,7 +144,7 @@ checksum = "1d07550c9036bf2ae0c684c4297d503f838287c83c53686d05370d0e139ae570"
 
 [[package]]
 name = "corsairctl"
-version = "0.1.2"
+version = "0.1.3"
 dependencies = [
  "alsa",
  "clap",
diff --git a/Cargo.toml b/Cargo.toml
index 8a16dab..bbcd0f0 100644
--- a/Cargo.toml
+++ b/Cargo.toml
@@ -1,6 +1,6 @@
 [package]
 name = "corsairctl"
-version = "0.1.2"
+version = "0.1.3"
 edition = "2024"
 description = "CLI tool for Corsair Bragi-protocol devices (HS80, etc.)"
 license = "MIT"
diff --git a/src/bragi/device.rs b/src/bragi/device.rs
index 19af5c6..e8ab86d 100644
--- a/src/bragi/device.rs
+++ b/src/bragi/device.rs
@@ -22,11 +22,7 @@ pub struct BragiDevice {
 
 impl BragiDevice {
     /// Findet ein Corsair-Gerät, öffnet es und führt die Init-Sequenz durch.
-    pub fn open() -> Result<Self> {
-        Self::open_with_verbose(false)
-    }
-
-    /// Wie `open()`, aber mit optionalem Debug-Output auf stderr.
+    /// Mit optionalem Debug-Output auf stderr.
     pub fn open_with_verbose(verbose: bool) -> Result<Self> {
         let api = HidApi::new()?;
         if verbose {
diff --git a/src/cli.rs b/src/cli.rs
index 0203c24..d3f75e6 100644
--- a/src/cli.rs
+++ b/src/cli.rs
@@ -29,6 +29,7 @@ pub enum Command {
     /// LED-Helligkeit lesen oder setzen (0-1000)
     Led {
         /// Helligkeit (0-1000). Ohne Angabe wird der aktuelle Wert gelesen.
+        #[arg(value_parser = clap::value_parser!(u16).range(0..=1000))]
         brightness: Option<u16>,
     },
 
diff --git a/src/main.rs b/src/main.rs
index 2b5c48a..85c1a47 100644
--- a/src/main.rs
+++ b/src/main.rs
@@ -33,9 +33,8 @@ fn run() -> error::Result<()> {
         Command::Led { brightness } => {
             let device = bragi::BragiDevice::open_with_verbose(cli.verbose)?;
             if let Some(value) = brightness {
-                let clamped = value.clamp(0, 1000);
-                device.set_brightness(clamped)?;
-                println!("{}", output::format_brightness(clamped));
+                device.set_brightness(value)?;
+                println!("{}", output::format_brightness(value));
             } else {
                 let current = device.brightness()?;
                 println!("{}", output::format_brightness(current));
diff --git a/tests/cli_test.rs b/tests/cli_test.rs
index c058d81..1a5e48f 100644
--- a/tests/cli_test.rs
+++ b/tests/cli_test.rs
@@ -44,3 +44,15 @@ fn sidetone_without_value_is_read_mode() {
     let result = parse_args(&["corsairctl", "sidetone"]);
     assert!(result.is_ok());
 }
+
+#[test]
+fn led_rejects_above_1000() {
+    let result = parse_args(&["corsairctl", "led", "1001"]);
+    assert!(result.is_err());
+}
+
+#[test]
+fn led_accepts_max() {
+    let result = parse_args(&["corsairctl", "led", "1000"]);
+    assert!(result.is_ok());
+}
diff --git a/tests/output_test.rs b/tests/output_test.rs
index dba4ce9..7b1f2d1 100644
--- a/tests/output_test.rs
+++ b/tests/output_test.rs
@@ -16,6 +16,30 @@ fn format_battery_charging() {
     assert_eq!(result, "Battery: 50% (Charging)");
 }
 
+#[test]
+fn format_battery_fully_charged() {
+    let result = output::format_battery(100.0, &BatteryStatus::FullyCharged);
+    assert_eq!(result, "Battery: 100% (Full)");
+}
+
+#[test]
+fn format_battery_low() {
+    let result = output::format_battery(10.0, &BatteryStatus::Low);
+    assert_eq!(result, "Battery: 10% (Low)");
+}
+
+#[test]
+fn format_battery_offline() {
+    let result = output::format_battery(0.0, &BatteryStatus::Offline);
+    assert_eq!(result, "Battery: 0% (Offline)");
+}
+
+#[test]
+fn format_battery_unknown() {
+    let result = output::format_battery(50.0, &BatteryStatus::Unknown(0x99));
+    assert_eq!(result, "Battery: 50% (Unknown)");
+}
+
 #[test]
 fn format_brightness_value() {
     let result = output::format_brightness(330);
@@ -125,6 +149,13 @@ fn waybar_json_class_at_boundary_30() {
     assert_eq!(parsed["class"], "warning");
 }
 
+#[test]
+fn waybar_json_class_unknown() {
+    let json_str = output::format_waybar_json(50.0, &BatteryStatus::Unknown(0x99), None);
+    let parsed: serde_json::Value = serde_json::from_str(&json_str).unwrap();
+    assert_eq!(parsed["class"], "unknown");
+}
+
 #[test]
 fn waybar_json_offline() {
     let json_str = output::format_waybar_json(0.0, &BatteryStatus::Offline, None);