Skip to content

Commit

Permalink
refactor: send response packet after context has been switched
Browse files Browse the repository at this point in the history
This refactor (at least partially) fixes a "mystery delay"
sometimes required after a dialog box swaps out. The previous
implementation would send the response, and then swap the contexts.
This would often cause the recipient to start drawing its UX
before the calling app was foregrounded, causing elements to go
missing.

This refactor puts the response after the context swap. The main
downside is it requires every type of modal to have a GAM
handle to issue the redraw call, and is especially complicated
by the fact that GAM is not clone-friendly. However, the
trade-offs of either creating a handle dynamically or storing
one in the object is probably worth the benefit of removing
an open-loop delay.
  • Loading branch information
bunnie committed Nov 6, 2022
1 parent b0f443b commit ffa8385
Show file tree
Hide file tree
Showing 9 changed files with 99 additions and 38 deletions.
11 changes: 2 additions & 9 deletions services/gam/src/modal.rs
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ pub trait ActionApi {
fn close(&mut self) {}
fn is_password(&self) -> bool { false }
/// navigation is one of '∴' | '←' | '→' | '↑' | '↓'
fn key_action(&mut self, _key: char) -> (Option<ValidatorErr>, bool) {(None, true)}
fn key_action(&mut self, _key: char) -> Option<ValidatorErr> {None}
fn set_action_opcode(&mut self, _op: u32) {}
}

Expand Down Expand Up @@ -478,16 +478,9 @@ impl<'a> Modal<'a> {
for &k in keys.iter() {
if k != '\u{0}' {
log::debug!("got key '{}'", k);
let (err, close) = self.action.key_action(k);
let err = self.action.key_action(k);
if let Some(err_msg) = err {
self.modify(None, None, false, Some(err_msg.to_str()), false, None);
} else {
if close {
// if it's a "close" button, invoke the GAM to put our box away
self.gam.relinquish_focus().unwrap();
xous::yield_slice();
break; // don't process any more keys after a close message
}
}
}
}
Expand Down
16 changes: 12 additions & 4 deletions services/gam/src/modal/bip39entry.rs
Original file line number Diff line number Diff line change
Expand Up @@ -298,7 +298,7 @@ impl ActionApi for Bip39Entry {
modal.gam.post_textview(&mut tv).expect("couldn't post tv");
}

fn key_action(&mut self, k: char) -> (Option<ValidatorErr>, bool) {
fn key_action(&mut self, k: char) -> Option<ValidatorErr> {
let can_move_downwards = !(self.suggestion_index.get() + 1 == NUM_RECCOS);
let can_move_upwards = !(self.suggestion_index.get() - 1 < 0);

Expand All @@ -310,9 +310,13 @@ impl ActionApi for Bip39Entry {
let mut ret = Bip39EntryPayload::default();
ret.data[..data.len()].copy_from_slice(&data);
ret.len = data.len() as u32;

// relinquish focus before returning the result
self.gam.relinquish_focus().unwrap();
xous::yield_slice();

let buf = Buffer::into_buf(ret).expect("couldn't convert message to payload");
buf.send(self.action_conn, self.action_opcode).map(|_| ()).expect("couldn't send action message");
return (None, true)
}
} else {
if self.suggested_words.len() > 0 {
Expand Down Expand Up @@ -345,10 +349,14 @@ impl ActionApi for Bip39Entry {
// ignore null messages
}
'\u{14}' => { // F4
// relinquish focus before returning the result
self.gam.relinquish_focus().unwrap();
xous::yield_slice();

let ret = Bip39EntryPayload::default(); // return a 0-length entry
let buf = Buffer::into_buf(ret).expect("couldn't convert message to payload");
buf.send(self.action_conn, self.action_opcode).map(|_| ()).expect("couldn't send action message");
return (None, true)
return None;
}
'\u{8}' => { // backspace
#[cfg(feature="tts")]
Expand Down Expand Up @@ -397,6 +405,6 @@ impl ActionApi for Bip39Entry {
}
}
}
(None, false)
None
}
}
12 changes: 9 additions & 3 deletions services/gam/src/modal/checkboxes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ pub struct CheckBoxes {
pub action_opcode: u32,
pub action_payload: CheckBoxPayload,
pub select_index: i16,
gam: crate::Gam,
#[cfg(feature = "tts")]
pub tts: TtsFrontend,
}
Expand All @@ -29,6 +30,7 @@ impl CheckBoxes {
action_opcode,
action_payload: CheckBoxPayload::new(),
select_index: 0,
gam: crate::Gam::new(&xous_names::XousNames::new().unwrap()).unwrap(),
#[cfg(feature="tts")]
tts,
}
Expand Down Expand Up @@ -141,7 +143,7 @@ impl ActionApi for CheckBoxes {
DrawStyle::new(PixelColor::Dark, PixelColor::Dark, 1))
).expect("couldn't draw entry line");
}
fn key_action(&mut self, k: char) -> (Option<ValidatorErr>, bool) {
fn key_action(&mut self, k: char) -> Option<ValidatorErr> {
log::trace!("key_action: {}", k);
match k {
'←' | '→' => {
Expand Down Expand Up @@ -180,9 +182,13 @@ impl ActionApi for CheckBoxes {
}
}
} else { // the OK button select
// relinquish focus before returning the result
self.gam.relinquish_focus().unwrap();
xous::yield_slice();

let buf = Buffer::into_buf(self.action_payload).expect("couldn't convert message to payload");
buf.send(self.action_conn, self.action_opcode).map(|_| ()).expect("couldn't send action message");
return (None, true)
return None;
}
}
'\u{0}' => {
Expand All @@ -192,6 +198,6 @@ impl ActionApi for CheckBoxes {
// ignore text entry
}
}
(None, false)
None
}
}
24 changes: 20 additions & 4 deletions services/gam/src/modal/consoleinput.rs
Original file line number Diff line number Diff line change
@@ -1,18 +1,30 @@
use crate::*;

/// This is a specialty structure that takes input from the serial console and records it to a string.
#[derive(Debug, Copy, Clone)]
#[derive(Debug)]
pub struct ConsoleInput {
pub action_conn: xous::CID,
pub action_opcode: u32,
pub action_payload: TextEntryPayload,
gam: Gam,
}
impl Clone for ConsoleInput {
fn clone(&self) -> Self {
ConsoleInput {
action_conn: self.action_conn,
action_opcode: self.action_opcode,
action_payload: self.action_payload,
gam: crate::Gam::new(&xous_names::XousNames::new().unwrap()).unwrap(),
}
}
}
impl ConsoleInput {
pub fn new(action_conn: xous::CID, action_opcode: u32) -> Self {
ConsoleInput {
action_conn,
action_opcode,
action_payload: TextEntryPayload::new(),
gam: crate::Gam::new(&xous_names::XousNames::new().unwrap()).unwrap(),
}
}
}
Expand All @@ -24,22 +36,26 @@ impl ActionApi for ConsoleInput {
fn redraw(&self, _at_height: i16, _modal: &Modal) {
// has nothing
}
fn key_action(&mut self, k: char) -> (Option<ValidatorErr>, bool) {
fn key_action(&mut self, k: char) -> Option<ValidatorErr> {
log::trace!("key_action: {}", k);
match k {
'\u{0}' => {
// ignore null messages
}
'∴' | '\u{d}' => {
// relinquish focus before returning the result
self.gam.relinquish_focus().unwrap();
xous::yield_slice();

let buf = Buffer::into_buf(self.action_payload).expect("couldn't convert message to payload");
buf.send(self.action_conn, self.action_opcode).map(|_| ()).expect("couldn't send action message");
return (None, true)
return None;
}
_ => { // text entry
self.action_payload.content.push(k).expect("ran out of space storing password");
log::trace!("****update payload: {}", self.action_payload.content);
}
}
(None, false)
None
}
}
12 changes: 9 additions & 3 deletions services/gam/src/modal/image.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ pub struct Image {
pub action_conn: xous::CID,
pub action_opcode: u32,
pub bitmap: Option<Bitmap>,
gam: crate::Gam,
}

impl Image {
Expand All @@ -15,6 +16,7 @@ impl Image {
action_conn,
action_opcode,
bitmap: None,
gam: crate::Gam::new(&xous_names::XousNames::new().unwrap()).unwrap(),
}
}
pub fn set_bitmap(&mut self, setting: Option<Bitmap>) {
Expand Down Expand Up @@ -48,21 +50,25 @@ impl ActionApi for Image {
.expect("couldn't draw bitmap");
}
}
fn key_action(&mut self, k: char) -> (Option<ValidatorErr>, bool) {
fn key_action(&mut self, k: char) -> Option<ValidatorErr> {
log::trace!("key_action: {}", k);
match k {
'\u{0}' => {
// ignore null messages
}
_ => {
// relinquish focus before returning the result
self.gam.relinquish_focus().unwrap();
xous::yield_slice();

send_message(
self.action_conn,
xous::Message::new_scalar(self.action_opcode as usize, 0, 0, 0, 0),
)
.expect("couldn't pass on dismissal");
return (None, true);
None
}
}
(None, false)
None
}
}
12 changes: 9 additions & 3 deletions services/gam/src/modal/notification.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ pub struct Notification {
pub manual_dismiss: bool,
pub qrcode: Vec<bool>,
pub qrwidth: usize,
gam: crate::Gam,
}
impl Notification {
pub fn new(action_conn: xous::CID, action_opcode: u32) -> Self {
Expand All @@ -28,6 +29,7 @@ impl Notification {
manual_dismiss: true,
qrcode: Vec::new(),
qrwidth: 0,
gam: crate::Gam::new(&xous_names::XousNames::new().unwrap()).unwrap(),
}
}
pub fn set_is_password(&mut self, setting: bool) {
Expand Down Expand Up @@ -184,23 +186,27 @@ impl ActionApi for Notification {
)
.expect("couldn't draw entry line");
}
fn key_action(&mut self, k: char) -> (Option<ValidatorErr>, bool) {
fn key_action(&mut self, k: char) -> Option<ValidatorErr> {
log::trace!("key_action: {}", k);
match k {
'\u{0}' => {
// ignore null messages
}
_ => {
// relinquish focus before returning the result
self.gam.relinquish_focus().unwrap();
xous::yield_slice();

send_message(
self.action_conn,
xous::Message::new_scalar(self.action_opcode as usize, k as u32 as usize, 0, 0, 0),
)
.expect("couldn't pass on dismissal");
if self.manual_dismiss {
return (None, true);
return None;
}
}
}
(None, false)
None
}
}
12 changes: 9 additions & 3 deletions services/gam/src/modal/radiobuttons.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ pub struct RadioButtons {
pub action_payload: RadioButtonPayload, // the current "radio button" selection
pub select_index: i16, // the current candidate to be selected
pub is_password: bool,
gam: crate::Gam,
#[cfg(feature = "tts")]
pub tts: TtsFrontend,
}
Expand All @@ -31,6 +32,7 @@ impl RadioButtons {
action_payload: RadioButtonPayload::new(""),
select_index: 0,
is_password: false,
gam: crate::Gam::new(&xous_names::XousNames::new().unwrap()).unwrap(),
#[cfg(feature="tts")]
tts,
}
Expand Down Expand Up @@ -152,7 +154,7 @@ impl ActionApi for RadioButtons {
DrawStyle::new(color, color, 1))
).expect("couldn't draw entry line");
}
fn key_action(&mut self, k: char) -> (Option<ValidatorErr>, bool) {
fn key_action(&mut self, k: char) -> Option<ValidatorErr> {
log::trace!("key_action: {}", k);
match k {
'←' | '→' => {
Expand All @@ -177,9 +179,13 @@ impl ActionApi for RadioButtons {
self.tts.tts_simple(self.items[self.select_index as usize].as_str()).unwrap();
}
} else { // the OK button select
// relinquish focus before returning the result
self.gam.relinquish_focus().unwrap();
xous::yield_slice();

let buf = Buffer::into_buf(self.action_payload).expect("couldn't convert message to payload");
buf.send(self.action_conn, self.action_opcode).map(|_| ()).expect("couldn't send action message");
return (None, true)
return None;
}
}
'\u{0}' => {
Expand All @@ -189,6 +195,6 @@ impl ActionApi for RadioButtons {
// ignore text entry
}
}
(None, false)
None
}
}
24 changes: 19 additions & 5 deletions services/gam/src/modal/slider.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,10 @@ use graphics_server::api::*;

use core::fmt::Write;

// This structure needs to be "shallow copy capable" so we can use it with
// the enum_actions API to update the progress state in an efficient manner.
// Thus it does not include its own GAM reference; instead we create one on
// the fly when needed.
#[derive(Debug, Copy, Clone)]
pub struct Slider {
pub min: u32,
Expand Down Expand Up @@ -163,7 +167,7 @@ impl ActionApi for Slider {
draw_list.push(GamObjectType::Rect(inner_rect)).unwrap();
modal.gam.draw_list(draw_list).expect("couldn't execute draw list");
}
fn key_action(&mut self, k: char) -> (Option<ValidatorErr>, bool) {
fn key_action(&mut self, k: char) -> Option<ValidatorErr> {
log::trace!("key_action: {}", k);
if !self.is_progressbar {
match k {
Expand All @@ -185,20 +189,30 @@ impl ActionApi for Slider {
// ignore null messages
}
'∴' | '\u{d}' => {
// relinquish focus before returning the result
let gam = crate::Gam::new(&xous_names::XousNames::new().unwrap()).unwrap();
gam.relinquish_focus().unwrap();
xous::yield_slice();

send_message(self.action_conn,
xous::Message::new_scalar(self.action_opcode as usize, self.action_payload as usize, 0, 0, 0)).expect("couldn't pass on action payload");
return(None, true)
return None;
}
_ => {
// ignore all other messages
}
}
(None, false)
None
} else {
if k == '🛑' { // use the "stop" emoji as a signal that we should close the progress bar
(None, true)
// relinquish focus on stop
let gam = crate::Gam::new(&xous_names::XousNames::new().unwrap()).unwrap();
gam.relinquish_focus().unwrap();
xous::yield_slice();

None
} else {
(None, false)
None
}
}
}
Expand Down
Loading

0 comments on commit ffa8385

Please sign in to comment.