feat(tweaker): refine control editing and address clippy warnings

CONTRIBUTING.md

@@ -13,7 +13,7 @@ When building a package for distribution, please ensure the following:
     ```
    The resulting binary will be located at `target/release/linux-enable-ir-emitter`. You can also use `cargo install --path <...>` to your convenience.
 4. The v7 is incompatible with the v6. If applicable, please make sure to use the provided [migration script]() on the saved configuration.
-   > [!Important]
+   > [!IMPORTANT]
    > This script is not yet available. It will be provided when the v7 will be officially released (currently in beta).
 
 ## Contributing Code
@@ -24,10 +24,8 @@ This project is using the usual Rust conventions. Here are some additional expla
    cargo build
     ```
    The resulting binary will be located at `target/debug/linux-enable-ir-emitter`
-
    > [!NOTE]
    > With a debug build, any camera can be used, even not infrared ones. This is useful for development and testing.
-
 2. Add the pre-commit hooks to make sure the linting checks and tests are passing before each commit:
    ```
    git config core.hooksPath .githooks

README.md

@@ -31,15 +31,15 @@ auth optional pam_exec.so /usr/local/bin/linux-enable-ir-emitter run --config /h
 ```
 
 > [!TIP]
-> The installation paths may vary depending on your installation method. You can determine the correct binary absolute paths by running `which linux-enable-ir-emitter` and use that path instead. For the configuration path, it will be written when you can execute `linux-enable-ir-emitter --config`.
+> The installation path may vary depending on your installation method. You can determine the correct binary absolute path by running `which linux-enable-ir-emitter` and use it instead. For the configuration path, it will be written when you execute `linux-enable-ir-emitter --config`.
 
 ### Integration with other program
 You will need to execute the `linux-enable-ir-emitter run` command before the program that uses the infrared camera.
 
 Alternatively, if you can and/or want to integrate better with the program that uses the camera, you can pass an opened file descriptor for the camera to the command: `linux-enable-ir-emitter run --device <DEVICE> --fd <FD>`.
 
-> [!Important]
-> You will need to pass the config path as argument to `linux-enable-ir-emitter run --config <CONFIG_PATH>` **when executed as root** if `linux-enable-ir-emitter configure` was executed as a normal user.
+> [!IMPORTANT]
+> You will need to pass the config path as argument to `linux-enable-ir-emitter run --config <CONFIG_PATH>` **when executed as root**, if `linux-enable-ir-emitter configure` was executed as a normal user.
 
 ## How do I enable my infrared emitter?
 0. For a better experience, use a large terminal window.

src/configure.rs

@@ -4,10 +4,12 @@ mod ui;
 pub async fn configure() -> anyhow::Result<()> {
     let res = app::run(&mut ratatui::init()).await;
     ratatui::restore();
+
+    // Print any successful message to the user once the TUI is closed
     if let Ok(msg) = &res
         && !msg.is_empty()
     {
         println!("{}", msg);
     }
-    res.map(|_| ())
+    res.map(|_| ()) // Delete the success message
 }

src/configure/app.rs

@@ -1,6 +1,7 @@
 mod helper;
 pub mod ir_enabler;
 pub mod tool_menu;
+pub mod tweaker;
 
 pub async fn run(terminal: &mut ratatui::DefaultTerminal) -> anyhow::Result<&'static str> {
     tool_menu::App::new().run(terminal).await

src/configure/app/ir_enabler.rs

@@ -1,6 +1,6 @@
 use super::helper::*;
 use crate::configure::ui::ir_enabler::{IrEnablerCtx, View, ui};
-use crate::configure::ui::keys::*;
+use crate::configure::ui::{DeviceSettingsCtx, SearchSettingsCtx};
 use crate::video::ir::analyzer::{
     self, IsIrWorking as AnalyzerResponse, Message as AnalyzerRequest, StreamAnalyzer,
 };
@@ -24,6 +24,14 @@ use tokio::{
     task,
 };
 
+const KEY_YES: KeyCode = KeyCode::Char('y');
+const KEY_NO: KeyCode = KeyCode::Char('n');
+const KEY_EXIT: KeyCode = KeyCode::Esc;
+const KEY_NAVIGATE_UP: KeyCode = KeyCode::Up;
+const KEY_NAVIGATE_DOWN: KeyCode = KeyCode::Down;
+const KEY_CONTINUE: KeyCode = KeyCode::Enter;
+const KEY_DELETE: KeyCode = KeyCode::Backspace;
+
 #[derive(Debug)]
 pub struct Config {
     /// Path to the video device.
@@ -334,7 +342,7 @@ impl App {
     async fn handle_term_event(&mut self, event: Event) -> Result<()> {
         match event {
             Event::Key(key_event) if key_event.kind == KeyEventKind::Press => {
-                self.handle_key_press(key_event.code.into()).await?;
+                self.handle_key_press(key_event.code).await?;
             }
             _ => {}
         };
@@ -422,13 +430,14 @@ impl App {
     }
 
     /// Handles a key event based on the current application state.
-    async fn handle_key_press(&mut self, key: Key) -> Result<()> {
+    async fn handle_key_press(&mut self, key: KeyCode) -> Result<()> {
         match self.state() {
             State::Menu => match key {
                 KEY_EXIT => self.set_state(State::Failure),
-                KEY_NAVIGATE => self.next_setting(),
+                KEY_NAVIGATE_UP => self.prev_setting(),
+                KEY_NAVIGATE_DOWN => self.next_setting(),
                 KEY_DELETE => self.edit_setting(None),
-                Key(KeyCode::Char(c)) => self.edit_setting(Some(c)),
+                KeyCode::Char(c) => self.edit_setting(Some(c)),
                 KEY_CONTINUE => self.set_state(State::ConfirmStart),
                 _ => {}
             },
@@ -458,7 +467,7 @@ impl App {
     /// In both of the two case, also changes the state to [`State::Running`].
     ///
     /// Otherwise, does nothing.
-    async fn confirm_working(&mut self, k: Key) -> Result<()> {
+    async fn confirm_working(&mut self, k: KeyCode) -> Result<()> {
         let mut response = IREnablerResponse::No;
         if k == KEY_YES {
             response = IREnablerResponse::Yes;
@@ -477,7 +486,7 @@ impl App {
     /// and sends [`IREnablerResponse::Abort`] to the configurator task.
     ///
     /// If the key is [`KEY_NO`], change the state back to [`State::Running`].
-    async fn abort_or_continue(&mut self, k: Key) -> Result<()> {
+    async fn abort_or_continue(&mut self, k: KeyCode) -> Result<()> {
         match k {
             KEY_NO | KEY_EXIT => self.set_state(self.prev_state()),
             KEY_YES => {
@@ -498,7 +507,7 @@ impl App {
     /// If the key is [`KEY_NO`], change the state back to the previous state.
     ///
     /// Returns directly an error if the video stream is already started.
-    fn start_or_back(&mut self, k: Key) -> Result<()> {
+    fn start_or_back(&mut self, k: KeyCode) -> Result<()> {
         // check that the path exists
         if !self.is_device_valid() {
             self.set_state(State::Menu);
@@ -534,15 +543,22 @@ impl App {
                 self.device_settings_list_state.select(None);
                 self.search_settings_list_state.select_first();
             }
-        } else if let Some(i) = self.search_settings_list_state.selected() {
-            if i < 3 {
-                self.search_settings_list_state.select_next();
+        } else {
+            self.search_settings_list_state.select_next();
+        }
+    }
+
+    /// Moves the selection to the previous setting in the settings lists.
+    fn prev_setting(&mut self) {
+        if let Some(i) = self.search_settings_list_state.selected() {
+            if i > 0 {
+                self.search_settings_list_state.select_previous();
             } else {
                 self.search_settings_list_state.select(None);
-                self.device_settings_list_state.select_first();
+                self.device_settings_list_state.select_last();
             }
         } else {
-            self.device_settings_list_state.select_first();
+            self.device_settings_list_state.select_previous();
         }
     }
 
@@ -594,6 +610,18 @@ impl IrEnablerCtx for App {
     fn show_menu_start_prompt(&self) -> bool {
         self.state() == State::ConfirmStart
     }
+    fn controls_list_state(&mut self) -> &mut ListState {
+        &mut self.controls_list_state
+    }
+    fn controls(&self) -> &[XuControl] {
+        &self.controls
+    }
+    fn image(&self) -> Option<&Image> {
+        self.image.as_ref()
+    }
+}
+
+impl DeviceSettingsCtx for App {
     fn device_settings_list_state(&mut self) -> &mut ListState {
         &mut self.device_settings_list_state
     }
@@ -615,6 +643,9 @@ impl IrEnablerCtx for App {
     fn fps(&self) -> Option<u32> {
         self.config.fps
     }
+}
+
+impl SearchSettingsCtx for App {
     fn search_settings_list_state(&mut self) -> &mut ListState {
         &mut self.search_settings_list_state
     }
@@ -633,15 +664,6 @@ impl IrEnablerCtx for App {
     fn inc_step(&self) -> u8 {
         self.config.inc_step
     }
-    fn controls_list_state(&mut self) -> &mut ListState {
-        &mut self.controls_list_state
-    }
-    fn controls(&self) -> &[XuControl] {
-        &self.controls
-    }
-    fn image(&self) -> Option<&Image> {
-        self.image.as_ref()
-    }
 }
 
 #[cfg(test)]
@@ -655,16 +677,16 @@ mod tests {
         App::new()
     }
 
-    fn make_key_event(keycode: Key) -> KeyEvent {
+    fn make_key_event(keycode: KeyCode) -> KeyEvent {
         KeyEvent {
-            code: keycode.into(),
+            code: keycode,
             modifiers: KeyModifiers::NONE,
             kind: KeyEventKind::Press,
             state: crossterm::event::KeyEventState::NONE,
         }
     }
 
-    fn make_term_key_event(keycode: Key) -> Event {
+    fn make_term_key_event(keycode: KeyCode) -> Event {
         Event::Key(make_key_event(keycode))
     }
 
@@ -713,7 +735,7 @@ mod tests {
     }
 
     #[test]
-    fn test_next_setting_device_to_search_and_back() {
+    fn test_next_setting_device_to_search() {
         let mut app = make_app();
         // Move through device settings (0..4)
         for i in 0..4 {
@@ -724,14 +746,47 @@ mod tests {
         app.next_setting();
         assert!(app.device_settings_list_state.selected().is_none());
         assert_eq!(app.search_settings_list_state.selected(), Some(0));
-        // Move through search settings (0..2)
+        // Move through search settings (0..3)
         for i in 0..3 {
             app.next_setting();
             assert_eq!(app.search_settings_list_state.selected(), Some(i + 1));
         }
-        // After 2, should wrap to device settings first
+        // After 3 we should stay at the last search setting
         app.next_setting();
+        assert_eq!(app.search_settings_list_state.selected(), Some(4));
+    }
+
+    #[test]
+    fn test_prev_setting_device_to_search() {
+        let mut app = make_app();
+        // Start at the end of search settings
+        app.device_settings_list_state.select(None);
+        app.search_settings_list_state.select(Some(4));
+
+        // Move through search settings (4..0)
+        for i in (1..=4).rev() {
+            assert_eq!(app.search_settings_list_state.selected(), Some(i));
+            app.prev_setting();
+        }
+
+        // At index 0 of search settings, prev_setting should move to device settings last
+        assert_eq!(app.search_settings_list_state.selected(), Some(0));
+        app.prev_setting();
         assert!(app.search_settings_list_state.selected().is_none());
+
+        // NOTE: select_last() sets to usize::MAX until screen is rendered to know that it is actually 4
+        assert_eq!(app.device_settings_list_state.selected(), Some(usize::MAX));
+        // so let's cheat
+        app.device_settings_list_state.select(Some(4));
+
+        // Move through device settings (4..0)
+        for i in (1..=4).rev() {
+            app.prev_setting();
+            assert_eq!(app.device_settings_list_state.selected(), Some(i - 1));
+        }
+
+        // At the first device setting, we should stay there
+        app.prev_setting();
         assert_eq!(app.device_settings_list_state.selected(), Some(0));
     }
 
@@ -880,7 +935,7 @@ mod tests {
         let mut app = make_app();
         app.set_state(State::Menu);
         app.device_settings_list_state.select(Some(0));
-        let key_event = make_term_key_event(KEY_NAVIGATE);
+        let key_event = make_term_key_event(KEY_NAVIGATE_DOWN);
         let res = app.handle_term_event(key_event).await;
         assert!(res.is_ok(), "{:?}", res.err());
         assert_eq!(app.device_settings_list_state.selected(), Some(1));

src/configure/app/tool_menu.rs

@@ -1,9 +1,9 @@
 use crate::configure::app::ir_enabler::App as IREnablerApp;
-use crate::configure::ui::keys::{KEY_CONTINUE, KEY_EXIT, KEY_NAVIGATE};
+use crate::configure::app::tweaker::App as TweakerApp;
 use crate::configure::ui::tool_menu::ui;
 
 use anyhow::Result;
-use crossterm::event;
+use crossterm::event::{self, KeyCode};
 use crossterm::event::{Event, KeyEventKind};
 
 /// Application state for the tool menu.
@@ -31,10 +31,10 @@ impl App {
             terminal.draw(|f| ui(f, self))?;
             match event::read()? {
                 Event::Key(key_event) if key_event.kind == KeyEventKind::Press => {
-                    match key_event.code.into() {
-                        KEY_NAVIGATE => self.next_tool(),
-                        KEY_CONTINUE => return self.start_tool(terminal).await,
-                        KEY_EXIT => return Ok(""),
+                    match key_event.code {
+                        KeyCode::Tab => self.next_tool(),
+                        KeyCode::Enter => return self.start_tool(terminal).await,
+                        KeyCode::Esc => return Ok(""),
                         _ => {}
                     }
                 }
@@ -57,7 +57,7 @@ impl App {
     ) -> Result<&'static str> {
         match self.state {
             State::IREnablerSelected => IREnablerApp::new().run(terminal).await,
-            State::UVCTweakerSelected => anyhow::bail!("UVC Tweaker is not yet implemented"),
+            State::UVCTweakerSelected => TweakerApp::new().run(terminal).await,
         }
     }
 }