Skip to content

Commit ffa8385

Browse files
committed
refactor: send response packet after context has been switched
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.
1 parent b0f443b commit ffa8385

File tree

9 files changed

+99
-38
lines changed

9 files changed

+99
-38
lines changed

services/gam/src/modal.rs

Lines changed: 2 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,7 @@ pub trait ActionApi {
5656
fn close(&mut self) {}
5757
fn is_password(&self) -> bool { false }
5858
/// navigation is one of '∴' | '←' | '→' | '↑' | '↓'
59-
fn key_action(&mut self, _key: char) -> (Option<ValidatorErr>, bool) {(None, true)}
59+
fn key_action(&mut self, _key: char) -> Option<ValidatorErr> {None}
6060
fn set_action_opcode(&mut self, _op: u32) {}
6161
}
6262

@@ -478,16 +478,9 @@ impl<'a> Modal<'a> {
478478
for &k in keys.iter() {
479479
if k != '\u{0}' {
480480
log::debug!("got key '{}'", k);
481-
let (err, close) = self.action.key_action(k);
481+
let err = self.action.key_action(k);
482482
if let Some(err_msg) = err {
483483
self.modify(None, None, false, Some(err_msg.to_str()), false, None);
484-
} else {
485-
if close {
486-
// if it's a "close" button, invoke the GAM to put our box away
487-
self.gam.relinquish_focus().unwrap();
488-
xous::yield_slice();
489-
break; // don't process any more keys after a close message
490-
}
491484
}
492485
}
493486
}

services/gam/src/modal/bip39entry.rs

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -298,7 +298,7 @@ impl ActionApi for Bip39Entry {
298298
modal.gam.post_textview(&mut tv).expect("couldn't post tv");
299299
}
300300

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

@@ -310,9 +310,13 @@ impl ActionApi for Bip39Entry {
310310
let mut ret = Bip39EntryPayload::default();
311311
ret.data[..data.len()].copy_from_slice(&data);
312312
ret.len = data.len() as u32;
313+
314+
// relinquish focus before returning the result
315+
self.gam.relinquish_focus().unwrap();
316+
xous::yield_slice();
317+
313318
let buf = Buffer::into_buf(ret).expect("couldn't convert message to payload");
314319
buf.send(self.action_conn, self.action_opcode).map(|_| ()).expect("couldn't send action message");
315-
return (None, true)
316320
}
317321
} else {
318322
if self.suggested_words.len() > 0 {
@@ -345,10 +349,14 @@ impl ActionApi for Bip39Entry {
345349
// ignore null messages
346350
}
347351
'\u{14}' => { // F4
352+
// relinquish focus before returning the result
353+
self.gam.relinquish_focus().unwrap();
354+
xous::yield_slice();
355+
348356
let ret = Bip39EntryPayload::default(); // return a 0-length entry
349357
let buf = Buffer::into_buf(ret).expect("couldn't convert message to payload");
350358
buf.send(self.action_conn, self.action_opcode).map(|_| ()).expect("couldn't send action message");
351-
return (None, true)
359+
return None;
352360
}
353361
'\u{8}' => { // backspace
354362
#[cfg(feature="tts")]
@@ -397,6 +405,6 @@ impl ActionApi for Bip39Entry {
397405
}
398406
}
399407
}
400-
(None, false)
408+
None
401409
}
402410
}

services/gam/src/modal/checkboxes.rs

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ pub struct CheckBoxes {
1616
pub action_opcode: u32,
1717
pub action_payload: CheckBoxPayload,
1818
pub select_index: i16,
19+
gam: crate::Gam,
1920
#[cfg(feature = "tts")]
2021
pub tts: TtsFrontend,
2122
}
@@ -29,6 +30,7 @@ impl CheckBoxes {
2930
action_opcode,
3031
action_payload: CheckBoxPayload::new(),
3132
select_index: 0,
33+
gam: crate::Gam::new(&xous_names::XousNames::new().unwrap()).unwrap(),
3234
#[cfg(feature="tts")]
3335
tts,
3436
}
@@ -141,7 +143,7 @@ impl ActionApi for CheckBoxes {
141143
DrawStyle::new(PixelColor::Dark, PixelColor::Dark, 1))
142144
).expect("couldn't draw entry line");
143145
}
144-
fn key_action(&mut self, k: char) -> (Option<ValidatorErr>, bool) {
146+
fn key_action(&mut self, k: char) -> Option<ValidatorErr> {
145147
log::trace!("key_action: {}", k);
146148
match k {
147149
'←' | '→' => {
@@ -180,9 +182,13 @@ impl ActionApi for CheckBoxes {
180182
}
181183
}
182184
} else { // the OK button select
185+
// relinquish focus before returning the result
186+
self.gam.relinquish_focus().unwrap();
187+
xous::yield_slice();
188+
183189
let buf = Buffer::into_buf(self.action_payload).expect("couldn't convert message to payload");
184190
buf.send(self.action_conn, self.action_opcode).map(|_| ()).expect("couldn't send action message");
185-
return (None, true)
191+
return None;
186192
}
187193
}
188194
'\u{0}' => {
@@ -192,6 +198,6 @@ impl ActionApi for CheckBoxes {
192198
// ignore text entry
193199
}
194200
}
195-
(None, false)
201+
None
196202
}
197203
}
Lines changed: 20 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,18 +1,30 @@
11
use crate::*;
22

33
/// This is a specialty structure that takes input from the serial console and records it to a string.
4-
#[derive(Debug, Copy, Clone)]
4+
#[derive(Debug)]
55
pub struct ConsoleInput {
66
pub action_conn: xous::CID,
77
pub action_opcode: u32,
88
pub action_payload: TextEntryPayload,
9+
gam: Gam,
10+
}
11+
impl Clone for ConsoleInput {
12+
fn clone(&self) -> Self {
13+
ConsoleInput {
14+
action_conn: self.action_conn,
15+
action_opcode: self.action_opcode,
16+
action_payload: self.action_payload,
17+
gam: crate::Gam::new(&xous_names::XousNames::new().unwrap()).unwrap(),
18+
}
19+
}
920
}
1021
impl ConsoleInput {
1122
pub fn new(action_conn: xous::CID, action_opcode: u32) -> Self {
1223
ConsoleInput {
1324
action_conn,
1425
action_opcode,
1526
action_payload: TextEntryPayload::new(),
27+
gam: crate::Gam::new(&xous_names::XousNames::new().unwrap()).unwrap(),
1628
}
1729
}
1830
}
@@ -24,22 +36,26 @@ impl ActionApi for ConsoleInput {
2436
fn redraw(&self, _at_height: i16, _modal: &Modal) {
2537
// has nothing
2638
}
27-
fn key_action(&mut self, k: char) -> (Option<ValidatorErr>, bool) {
39+
fn key_action(&mut self, k: char) -> Option<ValidatorErr> {
2840
log::trace!("key_action: {}", k);
2941
match k {
3042
'\u{0}' => {
3143
// ignore null messages
3244
}
3345
'∴' | '\u{d}' => {
46+
// relinquish focus before returning the result
47+
self.gam.relinquish_focus().unwrap();
48+
xous::yield_slice();
49+
3450
let buf = Buffer::into_buf(self.action_payload).expect("couldn't convert message to payload");
3551
buf.send(self.action_conn, self.action_opcode).map(|_| ()).expect("couldn't send action message");
36-
return (None, true)
52+
return None;
3753
}
3854
_ => { // text entry
3955
self.action_payload.content.push(k).expect("ran out of space storing password");
4056
log::trace!("****update payload: {}", self.action_payload.content);
4157
}
4258
}
43-
(None, false)
59+
None
4460
}
4561
}

services/gam/src/modal/image.rs

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ pub struct Image {
77
pub action_conn: xous::CID,
88
pub action_opcode: u32,
99
pub bitmap: Option<Bitmap>,
10+
gam: crate::Gam,
1011
}
1112

1213
impl Image {
@@ -15,6 +16,7 @@ impl Image {
1516
action_conn,
1617
action_opcode,
1718
bitmap: None,
19+
gam: crate::Gam::new(&xous_names::XousNames::new().unwrap()).unwrap(),
1820
}
1921
}
2022
pub fn set_bitmap(&mut self, setting: Option<Bitmap>) {
@@ -48,21 +50,25 @@ impl ActionApi for Image {
4850
.expect("couldn't draw bitmap");
4951
}
5052
}
51-
fn key_action(&mut self, k: char) -> (Option<ValidatorErr>, bool) {
53+
fn key_action(&mut self, k: char) -> Option<ValidatorErr> {
5254
log::trace!("key_action: {}", k);
5355
match k {
5456
'\u{0}' => {
5557
// ignore null messages
5658
}
5759
_ => {
60+
// relinquish focus before returning the result
61+
self.gam.relinquish_focus().unwrap();
62+
xous::yield_slice();
63+
5864
send_message(
5965
self.action_conn,
6066
xous::Message::new_scalar(self.action_opcode as usize, 0, 0, 0, 0),
6167
)
6268
.expect("couldn't pass on dismissal");
63-
return (None, true);
69+
None
6470
}
6571
}
66-
(None, false)
72+
None
6773
}
6874
}

services/gam/src/modal/notification.rs

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ pub struct Notification {
1818
pub manual_dismiss: bool,
1919
pub qrcode: Vec<bool>,
2020
pub qrwidth: usize,
21+
gam: crate::Gam,
2122
}
2223
impl Notification {
2324
pub fn new(action_conn: xous::CID, action_opcode: u32) -> Self {
@@ -28,6 +29,7 @@ impl Notification {
2829
manual_dismiss: true,
2930
qrcode: Vec::new(),
3031
qrwidth: 0,
32+
gam: crate::Gam::new(&xous_names::XousNames::new().unwrap()).unwrap(),
3133
}
3234
}
3335
pub fn set_is_password(&mut self, setting: bool) {
@@ -184,23 +186,27 @@ impl ActionApi for Notification {
184186
)
185187
.expect("couldn't draw entry line");
186188
}
187-
fn key_action(&mut self, k: char) -> (Option<ValidatorErr>, bool) {
189+
fn key_action(&mut self, k: char) -> Option<ValidatorErr> {
188190
log::trace!("key_action: {}", k);
189191
match k {
190192
'\u{0}' => {
191193
// ignore null messages
192194
}
193195
_ => {
196+
// relinquish focus before returning the result
197+
self.gam.relinquish_focus().unwrap();
198+
xous::yield_slice();
199+
194200
send_message(
195201
self.action_conn,
196202
xous::Message::new_scalar(self.action_opcode as usize, k as u32 as usize, 0, 0, 0),
197203
)
198204
.expect("couldn't pass on dismissal");
199205
if self.manual_dismiss {
200-
return (None, true);
206+
return None;
201207
}
202208
}
203209
}
204-
(None, false)
210+
None
205211
}
206212
}

services/gam/src/modal/radiobuttons.rs

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ pub struct RadioButtons {
1717
pub action_payload: RadioButtonPayload, // the current "radio button" selection
1818
pub select_index: i16, // the current candidate to be selected
1919
pub is_password: bool,
20+
gam: crate::Gam,
2021
#[cfg(feature = "tts")]
2122
pub tts: TtsFrontend,
2223
}
@@ -31,6 +32,7 @@ impl RadioButtons {
3132
action_payload: RadioButtonPayload::new(""),
3233
select_index: 0,
3334
is_password: false,
35+
gam: crate::Gam::new(&xous_names::XousNames::new().unwrap()).unwrap(),
3436
#[cfg(feature="tts")]
3537
tts,
3638
}
@@ -152,7 +154,7 @@ impl ActionApi for RadioButtons {
152154
DrawStyle::new(color, color, 1))
153155
).expect("couldn't draw entry line");
154156
}
155-
fn key_action(&mut self, k: char) -> (Option<ValidatorErr>, bool) {
157+
fn key_action(&mut self, k: char) -> Option<ValidatorErr> {
156158
log::trace!("key_action: {}", k);
157159
match k {
158160
'←' | '→' => {
@@ -177,9 +179,13 @@ impl ActionApi for RadioButtons {
177179
self.tts.tts_simple(self.items[self.select_index as usize].as_str()).unwrap();
178180
}
179181
} else { // the OK button select
182+
// relinquish focus before returning the result
183+
self.gam.relinquish_focus().unwrap();
184+
xous::yield_slice();
185+
180186
let buf = Buffer::into_buf(self.action_payload).expect("couldn't convert message to payload");
181187
buf.send(self.action_conn, self.action_opcode).map(|_| ()).expect("couldn't send action message");
182-
return (None, true)
188+
return None;
183189
}
184190
}
185191
'\u{0}' => {
@@ -189,6 +195,6 @@ impl ActionApi for RadioButtons {
189195
// ignore text entry
190196
}
191197
}
192-
(None, false)
198+
None
193199
}
194200
}

services/gam/src/modal/slider.rs

Lines changed: 19 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,10 @@ use graphics_server::api::*;
44

55
use core::fmt::Write;
66

7+
// This structure needs to be "shallow copy capable" so we can use it with
8+
// the enum_actions API to update the progress state in an efficient manner.
9+
// Thus it does not include its own GAM reference; instead we create one on
10+
// the fly when needed.
711
#[derive(Debug, Copy, Clone)]
812
pub struct Slider {
913
pub min: u32,
@@ -163,7 +167,7 @@ impl ActionApi for Slider {
163167
draw_list.push(GamObjectType::Rect(inner_rect)).unwrap();
164168
modal.gam.draw_list(draw_list).expect("couldn't execute draw list");
165169
}
166-
fn key_action(&mut self, k: char) -> (Option<ValidatorErr>, bool) {
170+
fn key_action(&mut self, k: char) -> Option<ValidatorErr> {
167171
log::trace!("key_action: {}", k);
168172
if !self.is_progressbar {
169173
match k {
@@ -185,20 +189,30 @@ impl ActionApi for Slider {
185189
// ignore null messages
186190
}
187191
'∴' | '\u{d}' => {
192+
// relinquish focus before returning the result
193+
let gam = crate::Gam::new(&xous_names::XousNames::new().unwrap()).unwrap();
194+
gam.relinquish_focus().unwrap();
195+
xous::yield_slice();
196+
188197
send_message(self.action_conn,
189198
xous::Message::new_scalar(self.action_opcode as usize, self.action_payload as usize, 0, 0, 0)).expect("couldn't pass on action payload");
190-
return(None, true)
199+
return None;
191200
}
192201
_ => {
193202
// ignore all other messages
194203
}
195204
}
196-
(None, false)
205+
None
197206
} else {
198207
if k == '🛑' { // use the "stop" emoji as a signal that we should close the progress bar
199-
(None, true)
208+
// relinquish focus on stop
209+
let gam = crate::Gam::new(&xous_names::XousNames::new().unwrap()).unwrap();
210+
gam.relinquish_focus().unwrap();
211+
xous::yield_slice();
212+
213+
None
200214
} else {
201-
(None, false)
215+
None
202216
}
203217
}
204218
}

0 commit comments

Comments
 (0)