hps-mon: Use rustyline's new external printer API
This lets us print output without messing up the prompt or any partially
written command, which means we no longer need to press enter in order
to activate the console.
BUG=b:247655226
TEST=./scripts/run-dev
Change-Id: I9154742c322de385b69e95b057fce5365a6fd865
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/hps-firmware/+/3941149
Reviewed-by: Jakub Młynarczyk <jakubm@chromium.org>
Tested-by: David Lattimore <dml@chromium.org>
Commit-Queue: David Lattimore <dml@chromium.org>
diff --git a/rust/hps-mon/src/console.rs b/rust/hps-mon/src/console.rs
deleted file mode 100644
index da72337..0000000
--- a/rust/hps-mon/src/console.rs
+++ /dev/null
@@ -1,83 +0,0 @@
-// Copyright 2021 The ChromiumOS Authors
-// Use of this source code is governed by a BSD-style license that can be
-// found in the LICENSE file.
-
-use std::sync::mpsc;
-use std::sync::Arc;
-use std::sync::Mutex;
-use std::thread::JoinHandle;
-
-/// Controls output to stdout, allowing it to be paused while the user is being
-/// prompted for input. This type can be cheaply cloned. All clones refer to the
-/// same underlying state.
-#[derive(Clone)]
-pub(crate) struct Console {
- /// Output requires locking this mutex, so is paused by holding the lock.
- output_lock: Arc<Mutex<()>>,
- line_sink: mpsc::Sender<String>,
-}
-
-pub(crate) struct ConsoleController {
- join_handle: JoinHandle<()>,
- console: Console,
-}
-
-impl ConsoleController {
- pub(crate) fn new() -> Self {
- let (line_sink, line_source) = mpsc::channel();
- let output_lock = Arc::new(Mutex::new(()));
- let join_handle = std::thread::spawn({
- let output_lock = output_lock.clone();
- move || {
- while let Ok(line) = line_source.recv() {
- let _guard = output_lock.lock().unwrap();
- println!("{}", line);
- }
- }
- });
- Self {
- join_handle,
- console: Console {
- output_lock,
- line_sink,
- },
- }
- }
-
- pub(crate) fn console(&self) -> Console {
- self.console.clone()
- }
-
- /// Shuts down the thread that handles printing to the console. All
- /// `Console` instances returned by `console` (or cloned from those) must
- /// have been dropped prior to calling this method.
- pub(crate) fn shutdown(self) {
- // There should be exactly 2 references to `output_lock`, one here and
- // one held by the thread created above.
- if Arc::strong_count(&self.console.output_lock) != 2 {
- panic!(
- "Call to ConsoleController::shutdown while there are still externally held \
- Console references"
- );
- }
- // We should have the only instance of Console, which means the only
- // `mpsc::Sender` that is linked to the `mpsc::Receiver` held by the
- // thread. We explicitly drop our console. Once the sender gets dropped,
- // the `recv()` call in the thread will return an error and the thread
- // will exit.
- drop(self.console);
- let _ = self.join_handle.join();
- }
-}
-
-impl Console {
- pub(crate) fn new_line_sink(&self) -> mpsc::Sender<String> {
- self.line_sink.clone()
- }
-
- /// Pauses console output while running `f`.
- pub(crate) fn pause_output_while<T, F: FnOnce() -> T>(&self, f: F) -> T {
- let _guard = self.output_lock.lock().unwrap();
- f()
- }
-}
diff --git a/rust/hps-mon/src/debug_commands.rs b/rust/hps-mon/src/debug_commands.rs
index be77ae4..2206fea 100644
--- a/rust/hps-mon/src/debug_commands.rs
+++ b/rust/hps-mon/src/debug_commands.rs
@@ -11,8 +11,8 @@
use mcu_common::DEBUG_BYTES_PER_COMMAND;
use rustyline::config::Configurer;
use std::sync::mpsc;
-
-use crate::console::Console;
+use std::sync::Arc;
+use std::sync::Mutex;
#[derive(Clone, Copy)]
pub(crate) struct DebugCommand {
@@ -82,6 +82,20 @@
should_exit: bool,
}
+#[derive(Clone)]
+pub(crate) struct ConsoleOutput {
+ external_printer: Arc<Mutex<dyn rustyline::ExternalPrinter>>,
+}
+
+impl ConsoleOutput {
+ pub(crate) fn print<T: Into<String>>(&self, out: T) {
+ // This will probably only fail if stdout gets closed. If that happens,
+ // then we don't have any useful way to report the error, so we ignore
+ // it.
+ let _ = self.external_printer.lock().unwrap().print(out.into());
+ }
+}
+
#[derive(Default)]
struct CompletionHelper {}
@@ -377,12 +391,12 @@
},
];
-/// Locks stdin and emits debug commands based on keys that get pressed.
-/// Terminates when the returned receiver gets closed.
-pub(crate) fn create_command_source(
- console: Console,
+/// Sets up console input and output. Returned values are a source of commands
+/// and a way to send output to the console. Terminates when the returned
+/// command receiver gets closed.
+pub(crate) fn create_console(
initial_commands: &str,
-) -> Result<mpsc::Receiver<Command>> {
+) -> Result<(mpsc::Receiver<Command>, ConsoleOutput)> {
let mut editor = CommandEditor {
editor: rustyline::Editor::<CompletionHelper>::new()?,
should_exit: false,
@@ -392,20 +406,22 @@
.editor
.set_completion_type(rustyline::CompletionType::List);
let (tx, rx) = mpsc::channel();
+ let console_output = ConsoleOutput {
+ external_printer: Arc::new(Mutex::new(editor.editor.create_external_printer()?)),
+ };
+ let outputs = (rx, console_output);
if let Ok(commands) = parse_commands(initial_commands) {
for command in commands {
let is_exit = matches!(command, Command::Exit);
if tx.send(command).is_err() || is_exit {
- return Ok(rx);
+ return Ok(outputs);
}
}
}
std::thread::spawn(move || {
- println!("Welcome to hps-mon. Press enter to activate console.");
+ println!("Welcome to hps-mon.");
loop {
- editor.wait_for_enter();
- let mut commands = vec![];
- console.pause_output_while(|| commands = editor.get_commands());
+ let commands = editor.get_commands();
for command in commands {
let is_exit = matches!(command, Command::Exit);
if tx.send(command).is_err() {
@@ -422,7 +438,7 @@
}
}
});
- Ok(rx)
+ Ok(outputs)
}
impl DebugCommand {
@@ -466,12 +482,6 @@
}
None
}
-
- fn wait_for_enter(&mut self) {
- if !self.should_exit {
- self.readline("");
- }
- }
}
fn parse_commands(commands_str: &str) -> Result<Vec<Command>, ()> {
diff --git a/rust/hps-mon/src/image_channel.rs b/rust/hps-mon/src/image_channel.rs
index 2d9395b..25973b4 100644
--- a/rust/hps-mon/src/image_channel.rs
+++ b/rust/hps-mon/src/image_channel.rs
@@ -2,13 +2,12 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
+use crate::debug_commands::ConsoleOutput;
use crate::ChannelHandler;
-use anyhow::anyhow;
use anyhow::Result;
use host_dev_common::images::ImageType;
use host_dev_common::images::OutputDir;
use std::path::Path;
-use std::sync::mpsc;
/// Receives images and writes them out to disk in RAW format and PNG.
pub(crate) struct ImageChannel {
@@ -20,14 +19,17 @@
pub(crate) fn new(
out_dir: &Path,
image_type: ImageType,
- line_sink: mpsc::Sender<String>,
+ console_output: ConsoleOutput,
) -> Result<Self> {
Ok(Self {
received: Vec::new(),
output_dir: OutputDir::new(
out_dir.to_owned(),
image_type,
- Box::new(move |log_line| line_sink.send(log_line).map_err(|e| anyhow!("{}", e))),
+ Box::new(move |log_line| {
+ console_output.print(log_line);
+ Ok(())
+ }),
)?,
})
}
diff --git a/rust/hps-mon/src/main.rs b/rust/hps-mon/src/main.rs
index 3e628cd..a07242f 100644
--- a/rust/hps-mon/src/main.rs
+++ b/rust/hps-mon/src/main.rs
@@ -2,7 +2,6 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
-mod console;
mod debug_commands;
mod image_channel;
mod openocd;
@@ -13,8 +12,7 @@
use anyhow::Result;
use clap::Parser;
use colored::*;
-use console::Console;
-use console::ConsoleController;
+use debug_commands::ConsoleOutput;
use host_dev_common::images::image_type_from_str;
use host_dev_common::images::ImageType;
use hps_interface::Hps;
@@ -201,38 +199,32 @@
Some(hps)
};
- let console_controller = ConsoleController::new();
- // Probably not strictly necessary, but we have an explicit scope to make
- // sure that `console` goes out of scope before we call `shutdown` on
- // `console_controller`.
- {
- let console = console_controller.console();
- let mut rtt = device.attach_rtt()?;
- let mut cmd_response_channel = None;
- if rtt.from_device.len() >= 4 {
- cmd_response_channel = rtt.from_device.drain(3..=3).next().and_then(|mut channel| {
- if channel.set_read_blocking(true).is_ok() {
- Some(channel)
- } else {
- None
- }
- });
- }
-
- let channels = create_channels(rtt.from_device, &flags, console.new_line_sink())?;
- let mut to_device = rtt.to_device.drain(..);
- run_loop(
- console,
- channels,
- to_device.next(),
- to_device.next(),
- cmd_response_channel,
- &flags,
- &mut *device,
- hps,
- )?;
+ let mut rtt = device.attach_rtt()?;
+ let mut cmd_response_channel = None;
+ if rtt.from_device.len() >= 4 {
+ cmd_response_channel = rtt.from_device.drain(3..=3).next().and_then(|mut channel| {
+ if channel.set_read_blocking(true).is_ok() {
+ Some(channel)
+ } else {
+ None
+ }
+ });
}
- console_controller.shutdown();
+
+ let (command_source, console_output) = debug_commands::create_console(&flags.cmd.join(";"))?;
+
+ let channels = create_channels(rtt.from_device, &flags, console_output)?;
+ let mut to_device = rtt.to_device.drain(..);
+ run_loop(
+ command_source,
+ channels,
+ to_device.next(),
+ to_device.next(),
+ cmd_response_channel,
+ &flags,
+ &mut *device,
+ hps,
+ )?;
Ok(())
}
@@ -255,20 +247,20 @@
fn create_channels(
mut from_device: Vec<Box<dyn DataSource>>,
flags: &Flags,
- line_sink: mpsc::Sender<String>,
+ console_output: ConsoleOutput,
) -> Result<Vec<Channel>> {
let mut raw_channels = from_device.drain(..);
let mut channels = Vec::new();
if let Some(up_channel) = raw_channels.next() {
channels.push(Channel::new(
up_channel,
- Box::new(TextChannel::new("MCU> ".yellow(), line_sink.clone())),
+ Box::new(TextChannel::new("MCU> ".yellow(), console_output.clone())),
)?);
}
if let Some(up_channel) = raw_channels.next() {
channels.push(Channel::new(
up_channel,
- Box::new(TextChannel::new("FPGA> ".green(), line_sink.clone())),
+ Box::new(TextChannel::new("FPGA> ".green(), console_output.clone())),
)?);
}
if let (Some(image_out), Some(up_channel)) = (&flags.image_out, raw_channels.next()) {
@@ -277,7 +269,7 @@
Box::new(image_channel::ImageChannel::new(
image_out,
flags.image_type,
- line_sink,
+ console_output,
)?),
)?);
}
@@ -285,7 +277,7 @@
}
fn run_loop(
- console: Console,
+ command_source: mpsc::Receiver<Command>,
mut channels: Vec<Channel>,
mut mcu_command_channel: Option<Box<dyn DataSink>>,
mut fpga_command_channel: Option<Box<dyn DataSink>>,
@@ -297,7 +289,6 @@
let mut backoff_ms = 0;
let mut buffer = vec![0; 1024];
set_blocking_mode(&mut mcu_command_channel, true)?;
- let command_source = debug_commands::create_command_source(console, &flags.cmd.join(";"))?;
loop {
let mut got_data = false;
for channel in &mut channels {
diff --git a/rust/hps-mon/src/text_channel.rs b/rust/hps-mon/src/text_channel.rs
index 85998ff..3f3e84e 100644
--- a/rust/hps-mon/src/text_channel.rs
+++ b/rust/hps-mon/src/text_channel.rs
@@ -2,8 +2,7 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
-use std::sync::mpsc;
-
+use crate::debug_commands::ConsoleOutput;
use crate::ChannelHandler;
use anyhow::Result;
use colored::ColoredString;
@@ -13,15 +12,15 @@
pub(crate) struct TextChannel {
received: Vec<u8>,
line_prefix: ColoredString,
- lines_out: mpsc::Sender<String>,
+ console_output: ConsoleOutput,
}
impl TextChannel {
- pub(crate) fn new(line_prefix: ColoredString, lines_out: mpsc::Sender<String>) -> Self {
+ pub(crate) fn new(line_prefix: ColoredString, console_output: ConsoleOutput) -> Self {
Self {
received: Vec::new(),
line_prefix,
- lines_out,
+ console_output,
}
}
}
@@ -38,12 +37,12 @@
{
if let Ok(message) = std::str::from_utf8(&self.received[..last_newline]) {
for line in message.lines() {
- self.lines_out
- .send(format!("{}{}", self.line_prefix, line))?;
+ self.console_output
+ .print(format!("{}{}", self.line_prefix, line));
}
self.received.drain(..last_newline + 1);
} else {
- self.lines_out.send("Device sent invalid UTF-8".into())?;
+ self.console_output.print("Device sent invalid UTF-8");
}
}
Ok(())