Skip to content

Add --host-command to send raw EC host commands#268

Merged
JohnAZoidberg merged 1 commit intomainfrom
host-command
Mar 2, 2026
Merged

Add --host-command to send raw EC host commands#268
JohnAZoidberg merged 1 commit intomainfrom
host-command

Conversation

@JohnAZoidberg
Copy link
Member

Useful for debugging new host commands

Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds a --host-command CLI option to send arbitrary Chromium EC host commands (ID, version, optional payload) and print the raw response, primarily for debugging new/unsupported EC commands.

Changes:

  • Replaces the unimplemented raw_command CLI field with host_command: Option<(u16, u8, Vec<u8>)> and wires it into execution via ec.send_command(...).
  • Implements parsing for --host-command in both UEFI argument parsing and the clap-based CLI.
  • Updates shell completions and advanced examples documentation to include --host-command.

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 6 comments.

Show a summary per file
File Description
framework_lib/src/commandline/uefi.rs Adds UEFI-shell parsing for --host-command, including hex/dec parsing helpers.
framework_lib/src/commandline/mod.rs Adds host_command to Cli and dispatches it to ec.send_command.
framework_lib/src/commandline/clap_std.rs Adds clap definition + conversion logic for --host-command.
completions/zsh/_framework_tool Adds zsh completion entry for --host-command.
completions/fish/framework_tool.fish Adds fish completion entry for --host-command.
completions/bash/framework_tool Adds bash completion flag for --host-command.
EXAMPLES_ADVANCED.md Documents usage and shows example outputs for raw EC host commands.

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

Comment on lines +1605 to +1606
} else if let Some((command_id, command_version, ref data)) = args.host_command {
match ec.send_command(command_id, command_version, data) {
Copy link

Copilot AI Mar 2, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

args is a shared reference (&Cli), but this pattern matches on args.host_command by value, which attempts to move the Option<(u16, u8, Vec<u8>)> out of a borrowed struct and won’t compile. Match on a reference instead (e.g. via args.host_command.as_ref() / &args.host_command) and borrow the payload bytes when calling send_command.

Suggested change
} else if let Some((command_id, command_version, ref data)) = args.host_command {
match ec.send_command(command_id, command_version, data) {
} else if let Some((command_id, command_version, data)) = args.host_command.as_ref() {
match ec.send_command(*command_id, *command_version, data) {

Copilot uses AI. Check for mistakes.
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

won’t compile

no it does build

Comment on lines +721 to +729
if let Some(byte) = parse_hex_or_dec_u8(&args[j]) {
data.push(byte);
} else {
println!("Invalid data byte for --host-command: '{}'", args[j]);
break;
}
}
Some((cmd_id, version, data))
} else {
Copy link

Copilot AI Mar 2, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When an invalid payload byte is encountered, the code prints an error and breaks, but still returns Some((cmd_id, version, data)) and will proceed to send a truncated payload. Treat any invalid byte as a hard parse failure for --host-command (return None / clear cli.host_command) so you don’t accidentally send a different command than intended.

Copilot uses AI. Check for mistakes.
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@copilot open a new pull request to apply changes based on this feedback

return 0
;;
--host-command)
COMPREPLY=($(compgen -f "${cur}"))
Copy link

Copilot AI Mar 2, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The bash completion treats --host-command like it expects a filename (compgen -f). This produces irrelevant suggestions and can be confusing since the arguments are numeric bytes/IDs. Remove the file completion for this flag (or provide numeric hints if desired).

Suggested change
COMPREPLY=($(compgen -f "${cur}"))
COMPREPLY=()

Copilot uses AI. Check for mistakes.
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are you sure? clap generates it like this

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hmm, other commands also have this problem, let's debug this later

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link

Copilot AI commented Mar 2, 2026

@JohnAZoidberg I've opened a new pull request, #269, to work on those changes. Once the pull request is ready, I'll request review from you.

@JohnAZoidberg JohnAZoidberg force-pushed the host-command branch 3 times, most recently from b795c8a to 37663cb Compare March 2, 2026 09:15
Useful for debugging new host commands

Signed-off-by: Daniel Schaefer <dhs@frame.work>
@JohnAZoidberg JohnAZoidberg merged commit d1a0710 into main Mar 2, 2026
7 checks passed
@JohnAZoidberg JohnAZoidberg deleted the host-command branch March 2, 2026 09:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants