Adjust LinearIO widgets without dedicated adjust inputs

[bartcerneels]

TL;DR Any LinearIO widget can get adjusted with next and prev buttons after an extra push of the select button.

I'm working on a micropython based firmware for a hacker camp badge. It's and ESP32 with st7789 LCD and I'm building the main menu and some installable apps using micro-gui.

I only have 3 inputs but found myself wanting to use the slider widget. Ideally any LinearIO would be usable for anyone wanting to make an app for this badge.

This patch adds a fallback for when Screen does not have "adjust" inputs such as an encoder or up/down switches.

I've tried it with all the demos I could run on my 240x240 display. The only weird effect is the "direction" of textbox scrolling: next scrolls up, not down.

[peterhinch]

I started out by doing some regression testing with a screen with two buttons and an encoder. I'm afraid this PR needs a good deal of work.

Firstly you've deleted the following files. Tests: audio.py, adjuster.py, adjust_vec.py, and listbox.py. The adjuster widget is also deleted.

After reinstating these files, the listbox.py demo fails immediately.

In all demos where precision mode can be invoked, when it is the Prev and Next buttons now adjust the value of the control. With an encoder they should behave normally i.e. move to another control.

The textbox demo occasionally fails with

Textbox demo.
Task exception wasn't retrieved
future: <Task> coro= <generator object '_run' at 2000feb0>
Traceback (most recent call last):
  File "uasyncio/core.py", line 1, in run_until_complete
  File "gui/primitives/encoder.py", line 69, in _run
  File "gui/core/ugui.py", line 232, in adjust
  File "gui/core/ugui.py", line 451, in do_adj
TypeError: function takes 3 positional arguments but 2 were given
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "gui/demos/tbox.py", line 120, in <module>
  File "gui/demos/tbox.py", line 118, in test
  File "gui/core/ugui.py", line 275, in change
  File "uasyncio/core.py", line 1, in run
  File "uasyncio/core.py", line 1, in run_until_complete

Line 451 looks wrong to me given the change to the move method.

At this point I'm afraid I lost heart. I do think the UI change is an interesting idea and I will give it some thought. Firstly I need to convince myself that your design approach is the best way to do this. I'd like to devise an approach with less potential for disrupting the existing code.

I don't know if you want to pursue this, or simply to fork the GUI for your badge project. If you do want to continue it needs to be tested in the existing configurations (encoder + 2 buttons, 5 buttons) as well as your 3-button setup.

[bartcerneels]

I don't remember deleting any files. And with the failures you are seeing I guess I didn't start from the right commit/branch. Let's fix that first.

[bartcerneels]

I don't know if you want to pursue this, or simply to fork the GUI for your badge project. If you do want to continue it needs to be tested in the existing configurations (encoder + 2 buttons, 5 buttons) as well as your 3-button setup.

I'll certainly use my fork as I've put quite some effort in already and it's working well enough. But upstreaming is always better and I'm glad to contribute time and skill.

In addition to 3 button I'm also using TouchPad support of the ESP32 (3 big pads of a Micro:bit connector) instead of switches. The hack to make this work is bad: replacing "Switch" with a new "Touch" primitive.

I currently don't have encoder or 5 button hardware but I'll dedicate a TTGO to it. For now I'll only test with what I have. The badge will get a "game-on" addon with regular buttons. The code will have to switch from touch to switch after reboot in hardware_setup.py

[bartcerneels]

At this point I'm afraid I lost heart. I do think the UI change is an interesting idea and I will give it some thought. Firstly I need to convince myself that your design approach is the best way to do this. I'd like to devise an approach with less potential for disrupting the existing code.

I don't like the current way either, it's a hack on top of already complicated input code. But since short press of select is not used yet, correct me if I'm wrong, it seems to enable adjusting with 3 buttons just fine.

For the touchpad support I mentioned I would abstract all input into a single class with callbacks for move, select and adjust. This code will also do the "debouncing" which currently seems to be done in Display::_closure(). Long press detection could also be done here, some hardware has that build in. It would make hardware_setup a bit more complicated, but ugui.py a whole lot simpler. For instance: there is no need to check for encoder in 'do_adj'.

Then for input without adjust, either Widget can forward move to do_adj as done in this PR or an Input::setAdjusting(True) gets called for adjustable widgets which makes it do 'adjust' callbacks instead of move.

[peterhinch]

I like the use of a short press on select. I just need to think about the implementation.

"debouncing" which currently seems to be done in Display::_closure()

Debouncing is done in this class

from gui.primitives.switch import Switch

Note that the Pushbutton class in primitives.pushbutton can detect double-clicks. I'll consider whether this could be used.

[bartcerneels]

Hey Peter, I believe I've fixed the issues you found. Rebased to current main and fixed the call of move with one arg.

The adjustment got triggered because precision does imply adjusting. Added a check for adjust input. As you can see, it makes the code even more complicated, hence my argument for abstracting it to Input.

Still have an issue with listbox (and menu), moving out is not possible. Perhaps using long press? More logical moving past the ends of the list should go prev/next in the screen.

I'll have an encoder to test with soon, turns out the game-on addon has it as an option.

[peterhinch]

I agree about abstracting the input. The present design evolved from a 2-5 button solution with encoder support and precision mode added later. I'm aware that it isn't pretty.

Moving from a listbox, a dropdown, and a menu could be tricky. A second select press is the logical solution, but there needs to be a way of "swallowing" that press to prevent triggering other actions. Alternatively it may be necessary to disallow certain widgets. My existing code, run with 2 or 3 buttons, only properly supports a limited set of widgets. As yet this isn't enforced, but as I'm sure you're aware if you instantiate (say) a slider, it will be good only for displaying a value.

If we're abstracting the input I would definitely look at the Pushbutton class rather than the simpler Switch. It's support for double clicks might solve the "swallowing" problem. It also supports long presses, possibly a better solution than the present code. Note the suppress constructor arg.

The encoder driver should preferably be kept as it solves a concurrency problem connected with the use of interrupts.

[peterhinch]

I've given this some more thought. I think we should start again based on the current master codebase. The following is a rough draft of a design.

The Input class

Input would be handled by an Input class, with select using the Pushbutton class. Input would intercept all input signals from buttons, encoders or any other potential source. It would deal with state changes, namely to and from precision or the new adjust status. Precision status would be entered by a long press and exited by any button press (as at present). Adjust would be entered and exited by a double-click. The Input class would communicate with Screen and the active widget using a single callback, which would have two args: event and status. The existing mechanism for routing the callback would be retained.

The Input class would handle the logic for the state changes. For example, in a 3-button system, precision mode would be unavailable. In a 5-button, or 3-button+encoder, system, adjust mode would be unavailable. The Pushbutton instance would simply not be provided with unwanted callbacks.

This means that Input would handle the double and long presses: the callback passed up the chain would have STATE_CHANGE and status args. Widgets would not "see" a Select button press because the Pushbutton class would not generate that callback. The STATE_CHANGE callback would enable a widget to change its appearance as required. This avoids any risk of a widget responding incorrectly to a Select press.

The Input status would be a single word bitfield containing the number of buttons, presence of an encoder, adjust and precision mode flags.

The effect of Next and Prev buttons would depend on the current state of the adjust bit. If set, UP or DOWN messages would be forwarded, otherwise NEXT or PREV. The widget does not need to "know" how UP and DOWN originate, although it has access to the adjust status bit.

I haven't fully thought through the way NEXT and PREV messages would be processed. The Screen class would handle changes in currency as at present, but there may be a need to forward this as a STATE_CHANGE to old and/or new widgets.

Trackpads

You mentioned a trackpad. My view is that this would be difficult to integrate with the current design which is based on relative rather than absolute measurements. Unless you have any specific ideas?

Implementation

The Input class may be best implemented as a base class with 3-button, 5-button, and Encoder subclasses. Or maybe not.

I'd be interested in your comments. I'm happy to look at implementing something along these lines if you're busy with other things.

[peterhinch]

I've made a start on this, separating the input into a separate Input class and using Pushbutton instead of Switch for the select button. The adjustment mode can be implemented by dynamically changing the callbacks for the Next and Prev buttons in response to a double click of Select. This keeps all the handling in the Input class so the widgets are unaware of the source of adjustments. It should also simplify combining adjustment and precision modes.

[peterhinch]

@bartcerneels I don't know if you're still following this issue but I'm making good progress with these changes.

I have concluded that using a push of select to provoke adjustment mode is not the way to do it. Pushing select provokes behaviour in widgets such as dropdown lists and menus; I also want to reserve this operation for other purposes. It is the obvious UI for launching callbacks from widgets or for logging values.

I am using a double-click of select to toggle adjustment mode which works well. It's also possible to use precision mode when using only three buttons with a long press of select.

Apologies for not using your code, but I very much appreciate your input in coming up with this useful idea. I will credit your contribution in the code and docs. Also this has prompted me to produce a more logical organisation of input, with mode changes handled in the Input class. The LinearIO class is substantially simplified relative to current master.

There is a lot more testing to do, but I hope to push an update in the next few days.

[bartcerneels]

Hey Peter

I have not been able to work on this the past few days. You certainly are a faster Python coder then me.

To clarify: I'm not intending to use a "trackpad" but rather the ESP32 capacitive touch. It works just as buttons.

Now that there is an Input class it's probably easier to integrate this with the main micro-gui codebase.

Double click to activate adjust is nice to have, but I do think it's harder to activate then a single click. I'll take a look at the code, perhaps I'll still patch to work with my 3-button setup. But I do wonder: if the single click of select is not used yet and you do have an "adjusting" mode: why not have at least the option to use single click?

Don't worry about not using my code, it's serviced it's purpose to demonstrate an idea.

[peterhinch]

You certainly are a faster Python coder then me.

A consequence of being retired and awful weather :)

A click of select is used by some widgets. For example here it triggers a callback. Menus and dropdowns are based on listboxes.

I'll let you know when I post an update.

[peterhinch]

I've pushed a first pass at this. There are a few other changes, such as simplified loading of widgets - see the examples. Changes are widespread so it's best to reload the gui tree (or use mpremote mount . as I do).

Re double-click - widgets such as Menu and Dropdown now automatically go into adjustment mode when activated. Numeric entry widgets do need the double-click.

Feel free to study/test/comment :)