fix: review changes

Signed-off-by: llogen <christoph.lange@blindspot.software>

Author: llogen

pkg/module/pikvm/README.md

@@ -2,11 +2,22 @@
 
 This module provides comprehensive control of a DUT via a PiKVM device. It offers power management through ATX control, keyboard input simulation, and virtual media mounting capabilities.
 
+PiKVM documentation: https://docs.pikvm.org/
+
+Compatibility: This module uses PiKVM's HTTP API endpoints (e.g. `/api/atx/*`, `/api/hid/*`, `/api/msd/*`, `/api/streamer/snapshot`). It is intended for PiKVM (kvmd/PiKVM OS) systems, and should work with other devices only if they expose a compatible API.
+
 ## Features
 
 ### Power Management
 Control the DUT's power state via ATX power and reset buttons:
 
+- Power commands use PiKVM's ATX control API:
+  - `on` is idempotent (does nothing if already powered on)
+  - `off` performs a graceful shutdown via power button press
+  - `force-off` performs a hard power off via long press (5+ seconds)
+  - `reset` triggers an ATX reset button press
+  - `force-reset` triggers a hardware hot reset
+
 ```
 COMMANDS:
   on           Power on (does nothing if already on)
@@ -20,16 +31,26 @@ COMMANDS:
 ### Keyboard Control
 Send keyboard input to the DUT:
 
+See [pikvm-key-strings.md](./pikvm-key-strings.md) for the full list of supported key strings.
+
 ```
 COMMANDS:
   type <text>          Type a text string
   key <keyname>        Send a single key (e.g., Enter, Escape, F12)
   key-combo <keys>     Send key combination (e.g., Ctrl+Alt+Delete)
+
+FLAGS (must come before the action):
+  --delay <duration>   Delay between key events for key-combo (default: 500ms)
 ```
 
 ### Virtual Media
 Mount ISO images or disk images as virtual USB devices:
 
+- `mount` uploads images to PiKVM's storage (with automatic space management)
+- `mount-url` instructs PiKVM to download the image directly from the URL
+- Old images are automatically deleted when storage space is needed
+- SHA256 hashing prevents duplicate uploads
+
 ```
 COMMANDS:
   mount <path>         Mount an image file from the agent's filesystem
@@ -38,130 +59,39 @@ COMMANDS:
   media-status         Show mounted media information
 ```
 
+### Screenshot
+Capture the current display output from PiKVM's video stream.
+
 ## Configuration Options
 
-| Option   | Type   | Default | Description                                                          |
-| -------- | ------ | ------- | -------------------------------------------------------------------- |
-| host     | string | -       | Address of the PiKVM device (e.g., "192.168.1.100")                 |
-| user     | string | admin   | Username for authentication                                          |
-| password | string | -       | Password for authentication                                          |
-| timeout  | string | 10s     | Timeout for HTTP requests (e.g., "10s", "30s")                      |
-| command  | string | -       | **Required**: Command type ("power", "keyboard", "media", "screenshot") |
+| Option   | Type   | Default | Description                                                                                                                                          |
+| -------- | ------ | ------- | ---------------------------------------------------------------------------------------------------------------------------------------------------- |
+| host     | string | -       | Address of the PiKVM device (e.g., "192.168.1.100") Defaults to HTTPS if no scheme is specified, HTTP can be used by explicitly specifying `http://` |
+| user     | string | admin   | Username for authentication                                                                                                                          |
+| password | string | -       | Password for authentication                                                                                                                          |
+| timeout  | string | 10s     | Timeout for HTTP requests (e.g., "10s", "30s")                                                                                                       |
+| mode     | string | -       | **Required**: Mode ("power", "keyboard", "media", "screenshot")                                                                                      |
 
 ⚠️ **Security Warning**: Passwords are stored in plaintext in the configuration file. This should only be used in trusted environments.
 
-## API Endpoints Used
-
-This module interacts with the following PiKVM API endpoints:
-
-- `/api/atx` - Get ATX power status
-- `/api/atx/power` - Intelligent power management (on/off/off_hard/reset_hard)
-- `/api/atx/click` - ATX button control (reset)
-- `/api/hid/print` - Type text input
-- `/api/hid/events/send_key` - Send keyboard keys and combinations
-- `/api/msd` - Mass Storage Device (virtual media) status
-- `/api/msd/write` - Upload images to PiKVM storage
-- `/api/msd/write_remote` - Download images from URL to PiKVM
-- `/api/msd/set_params` - Configure virtual media parameters
-- `/api/msd/set_connected` - Mount/unmount media
-- `/api/msd/remove` - Delete images from storage
-- `/api/streamer/snapshot` - Capture screenshot
-
 ## Usage Examples
 
 See [pikvm-example-cfg.yml](./pikvm-example-cfg.yml) for comprehensive configuration examples.
 
 ### Basic Power Control
-
-```yaml
-version: 0
-devices:
-  my-server:
-    desc: "Server controlled via PiKVM"
-    cmds:
-      power:
-        desc: "Power management: on|off|force-off|reset|force-reset|status"
-        modules:
-          - module: pikvm
-            main: true
-            options:
-              host: https://pikvm.local
-              user: admin
-              password: admin
-              command: power
-```
+See [pikvm-example-power.yml](./pikvm-example-power.yml).
 
 ### Boot Menu Access
-
-```yaml
-keyboard:
-  desc: "Keyboard control: type <text>|key <keyname>|key-combo <combo>"
-  modules:
-    - module: pikvm
-      main: true
-      options:
-        host: https://pikvm.local
-        user: admin
-        password: admin
-        command: keyboard
-
-# Usage: dutctl my-server keyboard key F12
-```
+See [pikvm-example-keyboard.yml](./pikvm-example-keyboard.yml).
 
 ### ISO Mounting
-
-```yaml
-media:
-  desc: "Virtual media control: mount <path>|mount-url <url>|unmount|media-status"
-  modules:
-    - module: pikvm
-      main: true
-      options:
-        host: https://pikvm.local
-        user: admin
-        password: admin
-        command: media
-
-# Usage: dutctl my-server media mount-url https://releases.ubuntu.com/22.04/ubuntu-22.04-live-server-amd64.iso
-```
+See [pikvm-example-media.yml](./pikvm-example-media.yml).
 
 ### Screenshot Capture
-
-```yaml
-screenshot:
-  desc: "Capture a screenshot from PiKVM"
-  modules:
-    - module: pikvm
-      main: true
-      options:
-        host: https://pikvm.local
-        user: admin
-        password: admin
-        command: screenshot
-
-# Usage: dutctl my-server screenshot
-# The screenshot will be saved to the current directory
-```
+See [pikvm-example-screenshot.yml](./pikvm-example-screenshot.yml).
 
 ## Requirements
 
 - PiKVM device with API access enabled
 - Network connectivity between dutagent and PiKVM
 - Valid authentication credentials
-
-## Notes
-
-- The module defaults to HTTPS if no scheme is specified in the host
-- HTTP can be used by explicitly specifying `http://` in the host
-- Power commands use intelligent API:
-  - `on` - Does nothing if already powered on (idempotent)
-  - `off` - Graceful shutdown via power button press
-  - `force-off` - Hard power off via long press (5+ seconds)
-  - `reset` - Reset button press
-  - `force-reset` - Hardware hot reset
-- Virtual media:
-  - `mount` uploads images to PiKVM's storage (with automatic space management)
-  - `mount-url` instructs PiKVM to download the image directly from the URL
-  - Old images are automatically deleted when storage space is needed
-  - SHA256 hashing prevents duplicate uploads
-- Screenshot functionality captures the current display output from PiKVM's video stream

pkg/module/pikvm/keyboard.go

@@ -16,30 +16,84 @@ import (
 	"github.com/BlindspotSoftware/dutctl/pkg/module"
 )
 
+// Keyboard command constants.
+const (
+	typeCmd  = "type"
+	key      = "key"
+	keyCombo = "key-combo"
+)
+
 const (
-	keyDelay           = 500 * time.Millisecond
 	maxTextLength      = 1024
 	minComboKeys       = 2
 	invalidCharNul     = "\x00"
 	invalidCharNewline = "\n"
 	invalidCharReturn  = "\r"
+	defaultKeyDelay    = 500 * time.Millisecond
 )
 
+func parseKeyboardFlags(args []string) (delay time.Duration, remaining []string, err error) {
+	delay = defaultKeyDelay
+
+	for len(args) > 0 {
+		arg := args[0]
+		if !strings.HasPrefix(arg, "--") {
+			break
+		}
+
+		if strings.HasPrefix(arg, "--delay=") {
+			value := strings.TrimPrefix(arg, "--delay=")
+			parsed, parseErr := time.ParseDuration(value)
+			if parseErr != nil || parsed <= 0 {
+				return 0, nil, fmt.Errorf("invalid --delay value %q", value)
+			}
+			delay = parsed
+			args = args[1:]
+			continue
+		}
+
+		if arg == "--delay" {
+			if len(args) < 2 {
+				return 0, nil, fmt.Errorf("--delay requires a duration value")
+			}
+			value := args[1]
+			parsed, parseErr := time.ParseDuration(value)
+			if parseErr != nil || parsed <= 0 {
+				return 0, nil, fmt.Errorf("invalid --delay value %q", value)
+			}
+			delay = parsed
+			args = args[2:]
+			continue
+		}
+
+		return 0, nil, fmt.Errorf("unknown flag %q", arg)
+	}
+
+	return delay, args, nil
+}
+
 // handleKeyboardCommandRouter routes keyboard commands based on the first argument.
 func (p *PiKVM) handleKeyboardCommandRouter(ctx context.Context, s module.Session, args []string) error {
-	if len(args) == 0 {
+	delay, remaining, err := parseKeyboardFlags(args)
+	if err != nil {
+		s.Printf("Error: %v\n", err)
+		s.Println("Usage: keyboard [--delay <duration>] <action> ...")
+		return nil
+	}
+
+	if len(remaining) == 0 {
 		s.Println("Keyboard command requires an action: type|key|key-combo")
 
 		return nil
 	}
 
-	command := strings.ToLower(args[0])
+	command := strings.ToLower(remaining[0])
 
-	return p.handleKeyboardCommand(ctx, s, command, args)
+	return p.handleKeyboardCommand(ctx, s, command, remaining, delay)
 }
 
 // handleKeyboardCommand dispatches keyboard-related commands.
-func (p *PiKVM) handleKeyboardCommand(ctx context.Context, s module.Session, command string, args []string) error {
+func (p *PiKVM) handleKeyboardCommand(ctx context.Context, s module.Session, command string, args []string, delay time.Duration) error {
 	switch command {
 	case typeCmd:
 		if len(args) < minArgsRequired {
@@ -64,7 +118,7 @@ func (p *PiKVM) handleKeyboardCommand(ctx context.Context, s module.Session, com
 			return nil
 		}
 
-		return p.handleCombo(ctx, s, args[1])
+		return p.handleCombo(ctx, s, args[1], delay)
 	default:
 		return fmt.Errorf("unknown keyboard action: %s (must be: type, key, key-combo)", command)
 	}
@@ -77,10 +131,8 @@ func (p *PiKVM) handleType(ctx context.Context, s module.Session, text string) e
 	}
 
 	// Check for invalid characters
-	for _, c := range []string{invalidCharNul, invalidCharNewline, invalidCharReturn} {
-		if strings.Contains(text, c) {
-			return fmt.Errorf("text contains invalid character: %q", c)
-		}
+	if idx := strings.IndexAny(text, invalidCharNul+invalidCharNewline+invalidCharReturn); idx != -1 {
+		return fmt.Errorf("text contains invalid character: %q", rune(text[idx]))
 	}
 
 	// Limit text length
@@ -91,7 +143,7 @@ func (p *PiKVM) handleType(ctx context.Context, s module.Session, text string) e
 	s.Printf("Typing text: %s\n", text)
 
 	// Use the /api/hid/print endpoint for text input
-	resp, err := p.doRequest(ctx, http.MethodPost, "/api/hid/print?slow=true", bytes.NewBufferString(text), "text/plain")
+	resp, err := p.doRequest(ctx, http.MethodPost, "/api/hid/print?slow=true", bytes.NewBufferString(text), "text/plain", requestOptions{})
 	if err != nil {
 		return fmt.Errorf("failed to type text: %v", err)
 	}
@@ -122,7 +174,7 @@ func (p *PiKVM) handleKey(ctx context.Context, s module.Session, keyName string)
 	return nil
 }
 
-func (p *PiKVM) handleCombo(ctx context.Context, s module.Session, comboStr string) error {
+func (p *PiKVM) handleCombo(ctx context.Context, s module.Session, comboStr string, delay time.Duration) error {
 	// Parse combo like "Ctrl+Alt+Delete" into array of keys
 	keys := strings.Split(comboStr, "+")
 	for idx := range keys {
@@ -145,7 +197,7 @@ func (p *PiKVM) handleCombo(ctx context.Context, s module.Session, comboStr stri
 	}
 
 	// Execute the key combination
-	err = p.executeKeyCombo(ctx, mappedModifiers, mainKeyMapped)
+	err = p.executeKeyCombo(ctx, mappedModifiers, mainKeyMapped, delay)
 	if err != nil {
 		return err
 	}
@@ -187,15 +239,15 @@ func (p *PiKVM) validateAndMapComboKeys(modifiers []string, mainKey string) (str
 }
 
 // executeKeyCombo executes a key combination by pressing modifiers, main key, and releasing modifiers.
-func (p *PiKVM) executeKeyCombo(ctx context.Context, modifiers []string, mainKey string) error {
+func (p *PiKVM) executeKeyCombo(ctx context.Context, modifiers []string, mainKey string, delay time.Duration) error {
 	// Press down all modifier keys
 	for _, modifier := range modifiers {
 		err := p.sendKeyRequest(ctx, fmt.Sprintf("/api/hid/events/send_key?key=%s&state=1", url.QueryEscape(modifier)))
 		if err != nil {
 			return err
 		}
 
-		time.Sleep(keyDelay)
+		time.Sleep(delay)
 	}
 
 	// Press and release the main key
@@ -204,7 +256,7 @@ func (p *PiKVM) executeKeyCombo(ctx context.Context, modifiers []string, mainKey
 		return err
 	}
 
-	time.Sleep(keyDelay)
+	time.Sleep(delay)
 
 	// Release all modifier keys in reverse order
 	for idx := len(modifiers) - 1; idx >= 0; idx-- {
@@ -213,15 +265,15 @@ func (p *PiKVM) executeKeyCombo(ctx context.Context, modifiers []string, mainKey
 			return err
 		}
 
-		time.Sleep(keyDelay)
+		time.Sleep(delay)
 	}
 
 	return nil
 }
 
 // sendKeyRequest sends a key request to PiKVM.
 func (p *PiKVM) sendKeyRequest(ctx context.Context, urlPath string) error {
-	resp, err := p.doRequest(ctx, http.MethodPost, urlPath, nil, "")
+	resp, err := p.doRequest(ctx, http.MethodPost, urlPath, nil, "", requestOptions{})
 	if err != nil {
 		return err
 	}