Skip to content

Conversation

@laurensvalk
Copy link
Member

@laurensvalk laurensvalk commented Jul 24, 2025

This enables the SPIKE Prime UI on EV3. We can design a more elaborate UI later.

This is meant to unblock progress on USB program download and run. All facets of downloading, running, and stopping programs become testable this way. Graceful shutdown can be tested by pressing and holding the center button. And by making it similar to SPIKE Prime, less cross platform testing is required.

There is more to be done here, like properly clearing the screen between program runs, but that isn't the priority right now.


A build flag ensures that attempting to start slot 0 on EV3 will always start the REPL so we have a way to keep testing until USB support is complete.

So to test download and run, use the left/right buttons to pick any other slot.


ui.mp4

The button HMI can still run. We just don't have to monitor the Bluetooth button.
This shouldn't call directly to the driver, but should use the system level pixel setter as it does everywhere else in this module.
A REPL is only useful while connected, so this option is only useful for work-in-progress builds like EV3 or NXT, so the REPL can be started from the UI even when there isn't any other communication protocol set up yet.
On platforms where the image module is disabled, we still need the image type for the static inline pbdrv_display_get_image.
This is not restricted to LED arrays.
This enables the SPIKE Prime UI on EV3 by mimicking a 5x5 grid display.

This unblocks the path to implementing USB download and run on EV3 without having to decide or spend time on the UI yet.

For now, attempting to run slot 0 will always start the UART REPL so that other developments can still be worked on in the mean time.
@laurensvalk
Copy link
Member Author

This is ready for use, but I suppose I need to check NXT builds now that we are working on that one too.

@BertLindeman
Copy link
Contributor

WOW

@coveralls
Copy link

Coverage Status

coverage: 56.982% (+0.1%) from 56.856%
when pulling aecbe20 on work
into 1c841c1 on master.

@laurensvalk
Copy link
Member Author

WOW

😄 I'm about to go into a meeting, but if you want to test that the NXT REPL still starts then we can merge it right after 😉

@BertLindeman
Copy link
Contributor

BertLindeman commented Jul 24, 2025

Great! UI on EV3!

NXT goes nice into the REPL.
I cannot really follow what is implemented where, but this PR is in master.

USB of EV3 is not recognized on linux and not on windows.
USB of NXT is recognized on linux but not on windows.

[EDIT]
No more REPL via S1 - serial. No USB (yet) no bluetooth (yet).
Oh wait.. a press of the center button gets me the serial REPL due to "No program there"?
Or is deliberately (for some testers) the REPL in slot 0 or 1 whichever start you choose.

@laurensvalk laurensvalk merged commit aecbe20 into master Jul 24, 2025
32 checks passed
@laurensvalk
Copy link
Member Author

laurensvalk commented Jul 24, 2025

Thanks for testing. Yep, NXT just remains unchanged for now.

It can get the same basic UI if we write a display driver though.

Or is deliberately (for some testers) the REPL in slot 0

Yep. Got to have something working for now.

Just one extra button press to get in.

@BertLindeman
Copy link
Contributor

Thanks Laurens,

The REPL in git hash 7c11ba2 is flaky.
As example:
in edit mode, paste

try:
    import gc
except ImportError:
    gc = None

try:
    import micropython
except ImportError:
    micropython = None

def meminfo():
    print("\nMemory info:")

    if gc:
        gc.collect()
        free = gc.mem_free()
        used = gc.mem_alloc()
        total = free + used
        print("\tTotal:", total, "bytes")
        print("\tFree :", free, "bytes")
        print("\tUsed :", used, "bytes")
    else:
        print("\tgc module not available")

    if micropython and hasattr(micropython, "mem_info"):
        print("\nDetailed info:\t")
        micropython.mem_info()
    else:
        print("\nmicropython.mem_info() not available")

meminfo()

Hangs after a few lines.
Will take the next firmware to test. (from pbio/platform/ev3: Enable 5x5 grid UI. #4054)

@laurensvalk
Copy link
Member Author

This has been the case for a while, prior to this change.

It appears that the UART operation hangs sometimes.

When this happened, USB was still available and happily going. (But the USB REPL has since been removed, so this is harder to verify.)

@laurensvalk
Copy link
Member Author

It's possible that the run animation makes it worse. This is a good thing since it makes it more reproducible. 😄

@BertLindeman
Copy link
Contributor

It's possible that the run animation makes it worse. This is a good thing since it makes it more reproducible. 😄

It is harder to do anything on the REPL.
If I can do anything to help, let me know.

@dlech dlech deleted the work branch July 24, 2025 23:35
@dlech
Copy link
Member

dlech commented Jul 26, 2025

It is harder to do anything on the REPL.

I made some hacks that seem to improve things a bit.

diff --git a/lib/pbio/drv/uart/uart_debug_first_port.c b/lib/pbio/drv/uart/uart_debug_first_port.c
index 8fe6e774a..85a21fced 100644
--- a/lib/pbio/drv/uart/uart_debug_first_port.c
+++ b/lib/pbio/drv/uart/uart_debug_first_port.c
@@ -13,13 +13,12 @@
 #include <stdarg.h>
 #include <string.h>
 
-#define BUF_SIZE (256)
 
-static uint8_t ring_buf[BUF_SIZE];
-static size_t ring_head = 0;
-static size_t ring_tail = 0;
+static uint8_t ring_buf[256 * 1024];
+static size_t ring_head;
+static size_t ring_tail;
 
-static pbdrv_uart_dev_t *debug_uart = NULL;
+static pbdrv_uart_dev_t *debug_uart;
 
 /**
  * Formats and stores a string in the UART debug ring buffer.
@@ -33,16 +32,16 @@ static pbdrv_uart_dev_t *debug_uart = NULL;
  */
 void pbdrv_uart_debug_printf(const char *format, ...) {
 
-    char buf[BUF_SIZE];
+    char buf[64];
     va_list args;
     va_start(args, format);
-    vsnprintf(buf, sizeof(ring_buf), format, args);
+    vsnprintf(buf, sizeof(buf), format, args);
     va_end(args);
 
     size_t len = strlen(buf);
     for (size_t i = 0; i < len; i++) {
         ring_buf[ring_head] = buf[i];
-        ring_head = (ring_head + 1) % BUF_SIZE;
+        ring_head = (ring_head + 1) % sizeof(ring_buf);
     }
 
     if (!debug_uart) {
@@ -86,10 +85,13 @@ static pbio_error_t pbdrv_uart_debug_process_thread(pbio_os_state_t *state, void
         PBIO_OS_AWAIT_UNTIL(state, ring_head != ring_tail);
 
         // Write up to the end of the buffer without wrapping.
-        size_t end = ring_head > ring_tail ? ring_head: BUF_SIZE;
+        size_t end = ring_head > ring_tail ? ring_head : sizeof(ring_buf);
         write_size = end - ring_tail;
+        if (write_size > 32) {
+            write_size = 32;
+        }
         PBIO_OS_AWAIT(state, &child, pbdrv_uart_write(&child, debug_uart, &ring_buf[ring_tail], write_size, 100));
-        ring_tail = (ring_tail + write_size) % BUF_SIZE;
+        ring_tail = (ring_tail + write_size) % sizeof(ring_buf);
 
         // Reset on failure.
         if (err != PBIO_SUCCESS) {
diff --git a/lib/pbio/drv/uart/uart_ev3.c b/lib/pbio/drv/uart/uart_ev3.c
index 86d4a7f75..0fa350ed5 100644
--- a/lib/pbio/drv/uart/uart_ev3.c
+++ b/lib/pbio/drv/uart/uart_ev3.c
@@ -61,7 +61,7 @@ struct _pbdrv_uart_dev_t {
     /** The current position in read_buf. */
     uint8_t read_pos;
     /** The buffer passed to the write function. */
-    uint8_t *write_buf;
+    uint8_t * volatile write_buf;
     /** The length of write_buf in bytes. */
     uint8_t write_length;
     /** The current position in write_buf. */

I made the ring_buf huge for debugging purposes, but we don't need to actually commit that part of the change. But it is still nice to decouple the size of the ring_buf from the size of the buf for formatting strings. 256 bytes is quite a lot of stack size for some of our smaller hubs.

But the key thing I changed that actually seemed to help is only writing a smallish number of bytes at a time to pbdrv_uart_write(). It is probably just hiding a deeper problem, but at least it gets things working for now.

@dlech dlech mentioned this pull request Jul 26, 2025
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.

5 participants