[Feature] redefine hub light status indications

[afarago]

Is your feature request related to a problem? Please describe.
The current system of indicating the hub state is not as clear as it could be.
I bumped into this especially with the bluetooth off indications.

Describe the solution you'd like
I would like to see a more concise way of handling lights for indicating non-user-program mode statuses.

• bluetooth status should be primarily indicated on the bluetooth light
• battery status should be primarily indicated on the battery light except for warnings
• status lights should only display warnings/errors

NOTE: when BT or battery light is not available (Move hub, City hub?) the status light should keep the BT/BATTERY status as well - per or even extending the current implementation.

STATUS LIGHT
connected	none
disconnected	none
high current	flashing orange pat.1
low voltage	flashing orange pat.2
shutting down	alternating blue/none
BLUETOOTH LIGHT
connected	none
disconnected	flashing blue
disabled	red
low signal	flashing blue-yellow currently not available in firmware
BATTERY LIGHT
charging	red
charging full	green
discharging normal	none
discharging medium voltage	yellow
discharging low voltage	red

Describe alternatives you've considered
The current coloring is problematic especially when using in WRO or FLL competitions.
It is always OK to set it to disabled, yet if that does not happen the BT=BLUE suggests that the hub is connected over BT to a computer. This is to be avoided.

Additional context
Add any other context or screenshots about the feature request here.

I have played around a bit and this is a preliminary implementation for demo purposes.
https://github.com/afarago/pybricks-micropython/tree/feature/lights-redefined

[laurensvalk]

Thanks for opening this issue.

As noted in the issue about disabling Bluetooth, the entire hub UI probably needs to be revisited at some point. This is why that feature hasn't been released yet.

Having multi-program support, a port view program, USB support, and the UI on other hubs might all be part of that consideration.

[laurensvalk]

Here's an alternate idea for the time being that might help us get the Bluetooth toggle in a release state in a reasonable time frame:

The current coloring is problematic especially when using in WRO or FLL competitions.
It is always OK to set it to disabled, yet if that does not happen the BT=BLUE suggests that the hub is connected over BT to a computer. This is to be avoided.

So instead of having lights around the Bluetooth button as we added recently, how about just turning off the blue center button light if Bluetooth is disabled? Since this only affects the Prime/Inventor Hub, you can still tell that a program is or isn't running using the matrix display.

It seems like this might address the concerns for the time being, without having to introduce an all-new UI just yet. I'll try making a PR for this later this week to see how it looks.

What do you think?

[laurensvalk]

So instead of having lights around the Bluetooth button as we added recently, how about just turning off the blue center button light if Bluetooth is disabled? Since this only affects the Prime/Inventor Hub, you can still tell that a program is or isn't running using the matrix display.

Looking at the code, it looks like the solid blue is not an indication pattern but a default when there isn't anything else to indicate.

So perhaps the right way to implement this is to introduce dedicated patterns for IDLE_CONNECTED (solid blue) with variants for low battery and so on. Then the fallback for no indication it to show nothing at all instead.

The quick hacky way to achieve the same thing is to make sure this fallback only runs when connected: The proper solution is simple enough and doesn't take all that much space.

[laurensvalk]

@afarago , pybricks/pybricks-micropython#261 implements the above suggestion. How does this look when you try it?

[afarago]

Thanks for the progress.

• I love that the there is either off or blue flashing.
• It is very cumbersome and hard to follow that pressing the bluetooth button controls the light of the main button and not the BT button itself.

1. Would still suggest to move the BT connection state indication to the BT button.
2. Alternatively this extension might help to understanding the state being turned off

BLUETOOTH LIGHT
connected	none
disconnected (broadcasting)	flashing blue
disabled	flashes (1x / tbd more) red then off

[afarago]

Adding as a reminder. When BT is off user will not be able to connect to the hub. (Captain Obvious)

A reminder to try pushing the BT button if broadcast is not indicated would be helpful in the IDE.

[dlech]

I like Attila's suggestion of using the Bluetooth light (if there is one) for all things Bluetooth. Except for the part where light off means both connected and disabled. So I would suggest solid blue on for connected.

In other words, if the hub has a Bluetooth button/light, just move the current Bluetooth aspects of the light UI from the status button light to the Bluetooth button light.

That is, make this change and add a new similar function for the Bluetooth status light.

diff --git a/lib/pbio/sys/light.c b/lib/pbio/sys/light.c
index 6bc15d7b4..06a34bdab 100644
--- a/lib/pbio/sys/light.c
+++ b/lib/pbio/sys/light.c
@@ -191,16 +191,20 @@ static void pbsys_status_light_handle_status_change(void) {
         new_indication = PBSYS_STATUS_LIGHT_INDICATION_SHUTDOWN;
     } else if (shutdown_requested) {
         new_indication = PBSYS_STATUS_LIGHT_INDICATION_SHUTDOWN_REQUESTED;
+#if !PBSYS_CONFIG_STATUS_LIGHT_BLUETOOTH
     } else if (ble_advertising && low_voltage) {
         new_indication = PBSYS_STATUS_LIGHT_INDICATION_BLE_ADVERTISING_AND_LOW_VOLTAGE;
     } else if (ble_advertising) {
         new_indication = PBSYS_STATUS_LIGHT_INDICATION_BLE_ADVERTISING;
+#endif
     } else if (high_current) {
         new_indication = PBSYS_STATUS_LIGHT_INDICATION_HIGH_CURRENT;
+#if !PBSYS_CONFIG_STATUS_LIGHT_BLUETOOTH
     } else if (ble_low_signal && low_voltage) {
         new_indication = PBSYS_STATUS_LIGHT_INDICATION_BLE_LOW_SIGNAL_AND_LOW_VOLTAGE;
     } else if (ble_low_signal) {
         new_indication = PBSYS_STATUS_LIGHT_INDICATION_BLE_LOW_SIGNAL;
+#endif
     } else if (low_voltage) {
         new_indication = PBSYS_STATUS_LIGHT_INDICATION_LOW_VOLTAGE;
     }

[laurensvalk]

For now the point isn't to come up with the final UI, but something that lets us release it in beta at all. Otherwise, I think we should just disable the feature altogether for the foreseeable future.

This feature is needed for a very small number of users (and for the wrong reasons, since Bluetooth doesn't have to be disabled) but impacts everyone. I'm happy to include something passable for now, but I would rather not change too much right now.

The final light UI has to include things like USB handling, multi programs, and port view, and consistency across all hubs, so I'd like to keep the options open.

[afarago]

I fear that the simple implementation pybricks/pybricks-micropython#261 might cause more confusion than solving issues, but honestly cannot judge. Might be a good call to invite the community to express their opinion.

Let me know if I can contribute and help by implementing or defining any of the above to speed things up.

Most teams today do without disabling the BT fine, so there should be no direct rush with it.
Disabling BT is a pretty obtrusive action, that should have proper feedback (e.g. light indication / red flashing x times or a beep), and also might come with a safety mechanism (2 sec hold to disable).

I am pretty sure that disabling bluetooth is just a workaround to resolve a privacy/protection topic.
The real user need is imo controlling the ability to connect to the hub. Here LEGO stock firmware found a good solution -> enable new connections by pressing the BT button and then being able to connect from the device you already paired up with.
This requires keeping the UUID stable though #1615 (comment).
I am not sure how it works in practice though (BT must announce to be able to connect, right?; but then how others not seeing the hub announce itself)

Should I raise a separate feature request or a discussion for this? (keeping UUID stable & closed-BT & UX for one-time-open-connection)?

[laurensvalk]

Most teams today do without disabling the BT fine, so there should be no direct rush with it.

I had a feeling a few teams wanted it for this seasons already, so that's why I thought it would be nice to have something in place. Then people may not feel like they have to use custom builds from outdated pull requests (we had a question about that just yesterday), which can sometimes be a recipe for trouble.

Perhaps as a reasonable alternative, we can keep the build flag disabled by default, but add an article to the site with a link to the correct custom build with that feature enabled.

Should I raise a separate feature request or a discussion for this? (keeping UUID stable & closed-BT & UX for one-time-open-connection)?

I'm afraid to say that this particular change probably wouldn't happen any time soon due to lack of time.

But turning Bluetooth off (assuming we figure out a good UI), and potentially adding USB support in the future, seems to be a fairly reasonable path forward that covers most scenarios?

[laurensvalk]

In case we are going to move half of the light indications to the bluetooth button, what are your preferences for the main hub status light? Should it still glow/fade blue when a program is running, given that we already have an animation on the hub display?

We'll also want to keep the familiarity aspect in mind. So far, white = SPIKE V2, green = SPIKE V3, blue = Pybricks. This wouldn't apply anymore. Or we do let it remain blue, even when Bluetooth is off, since the button light hopefully makes it clear then.

[BertLindeman]

Should it still glow/fade blue when a program is running, given that we already have an animation on the hub display?

My 2 cents: I would like the indication on the power button to be kept. If the display matrix is used for something else, the status would be lost.

[afarago]

In case we are going to move half of the light indications to the bluetooth button, what are your preferences for the main hub status light? Should it still glow/fade blue when a program is running, given that we already have an animation on the hub display?

We'll also want to keep the familiarity aspect in mind. So far, white = SPIKE V2, green = SPIKE V3, blue = Pybricks. This wouldn't apply anymore. Or we do let it remain blue, even when Bluetooth is off, since the button light hopefully makes it clear then.

Idle = Blue
Running = Off
would combine the two imo.

[afarago]

I'm afraid to say that this particular change probably wouldn't happen any time soon due to lack of time.

How about adding a small detail for combining both.

Bluetooth long press = generate new UUID

otherwise keep using the current one.

Normally GATT sevice set is unchanged anyhow, and for any protocol change or other this would add an easy troubleshooting.

---

If this sounds reasonable enough next week I can play around with the code and come up with a discussion-ready-quality PR.

I assume currenty the UUID is not persisted yet.

[laurensvalk]

I'm afraid to say that this particular change probably wouldn't happen any time soon due to lack of time.

Bluetooth long press = generate new UUID (...) otherwise keep using the current one

Just for context, the intended security aspect probably does deserve a dedicated issue even if we might not get to it soon. Keeping all the ideas together is always nice.

For now the point isn't to come up with the final UI, but something that lets us release it in beta at all. Otherwise, I think we should just disable the feature altogether for the foreseeable future.

If we finish pybricks/pybricks-micropython#264, reach consensus on #139 and implement this UI, and implement multi program storage (there are some relatively simple approaches to get started), then maybe a combined UI isn't all that far off, and we could potentially do it all at once 😄

And then the solution for someone in a hurry will be to use the latest master build.

[laurensvalk]

I think I am going to refactor the light patterns from

typedef enum {
    PBSYS_STATUS_LIGHT_INDICATION_NONE,
    PBSYS_STATUS_LIGHT_INDICATION_HIGH_CURRENT,
    PBSYS_STATUS_LIGHT_INDICATION_LOW_VOLTAGE,
    PBSYS_STATUS_LIGHT_INDICATION_BLE_ADVERTISING,
    PBSYS_STATUS_LIGHT_INDICATION_BLE_ADVERTISING_AND_LOW_VOLTAGE,
    PBSYS_STATUS_LIGHT_INDICATION_BLE_LOW_SIGNAL,
    PBSYS_STATUS_LIGHT_INDICATION_BLE_LOW_SIGNAL_AND_LOW_VOLTAGE,
    PBSYS_STATUS_LIGHT_INDICATION_BLE_CONNECTED_IDLE,
    PBSYS_STATUS_LIGHT_INDICATION_BLE_CONNECTED_IDLE_AND_LOW_VOLTAGE,
    PBSYS_STATUS_LIGHT_INDICATION_SHUTDOWN_REQUESTED,
    PBSYS_STATUS_LIGHT_INDICATION_SHUTDOWN,
} pbsys_status_light_indication_t;

to:

typedef enum {
    PBSYS_STATUS_LIGHT_INDICATION_BLUETOOTH_BLE_OFF,
    PBSYS_STATUS_LIGHT_INDICATION_BLUETOOTH_BLE_ADVERTISING,
    // Skipping low signal since this wasn't actually used anywhere. But could be added back here.
    PBSYS_STATUS_LIGHT_INDICATION_BLUETOOTH_BLE_CONNECTED_IDLE,
} pbsys_status_light_indication_bluetooth_t;

typedef enum {
    PBSYS_STATUS_LIGHT_INDICATION_WARNING_NONE,
    PBSYS_STATUS_LIGHT_INDICATION_WARNING_HIGH_CURRENT,
    PBSYS_STATUS_LIGHT_INDICATION_WARNING_LOW_VOLTAGE,
    PBSYS_STATUS_LIGHT_INDICATION_WARNING_SHUTDOWN_REQUESTED,
    PBSYS_STATUS_LIGHT_INDICATION_WARNING_SHUTDOWN,
} pbsys_status_light_indication_warning_t;

On the hubs with one light, warnings will take priority and overlay/override appropriately. On hubs with a separate Bluetooth light, they'll be displayed separately.

I would also like to propose to use a single orange pattern for low battery, instead of two differently timed orange patterns we have now.

[dlech]

If we are going to be doing a major status light overhaul, there is one thing that I have always missed and I think could also help with support requests.

I would like to have an indication when the user program ends that lets the user know why it ended:

1. Reached end of program (includes raising SystemExit - or maybe all BaseException like KeyboardInterrupt too?)
2. Program aborted due to unhandled exception.
3. Stop button pressed (not sure if this one deserves a separate indication - or no indication?)

This is especially helpful when running a program while not connected to a computer.

And we frequently get support requests where we have to ask to see the whole program and try to understand the program to see what might be happening. But with some sort of status light indication, we could probably help most people without actually having to dig into the program.

Not sure what indications would be intuitive to people though. At least one version of the official LEGO SPIKE Prime firmware blinked red a few times on unhandled exception, which seemed nice to me.

[laurensvalk]

Yes, I think we can do that. It seems like we could do that with one or more additional status flags (`did_error_x), and a non-repeating light pattern to indicate it. This way, we shouldn't have to worry about waiting for the light signal to complete, since starting a new program will take care of clearing that status and light pattern.

I suppose any normal exit including the stop button do not need indications, but unhandled exceptions could certainly have one. If we want to distinguish further, I think we could have one indication for ENODEV and then one for every other unhandled exception. On Prime Hub, maybe ENODEV could have a matching animation highlighting the bad port.

---

Also to be complete, here is something from 4 years ago, which is the slightly more elaborate version of #139. I'm not as much of a fan of this iteration anymore though.

[laurensvalk]

Quite a bit has been done in pybricks/pybricks-micropython#261.

A few ideas came up in conversation the other day, and then later in posts above:

• To make it absolutely clear that Bluetooth is not only off but also not available for remote controls, we could consider making the Bluetooth light red after all (as we had in various intermediate iterations).
• We could consider an indication for scanning for a peripheral (remote) or being connected to one. Could be nice, but maybe a bit much. Or maybe this could be done on the hub light, as part of the Remote and XboxController classes instead? Or maybe it should be left to the user?
• If we have USB support in the future, maybe the Bluetooth light could turn green if it is connected to a host via USB instead of Bluetooth.

[dlech]

I suppose any normal exit including the stop button do not need indications

Maybe. One of the cases I was thinking about was the frequent support requests that, "I ran my program and it didn't do anything". If there was a light pattern on successful completion, then we would know that someone either wrote functions and didn't call them or didn't call any blocking functions.

Then we could write a self-help section in the docs that "if you see this light pattern and your program didn't do anything then you probably made one of these common mistakes..."

Having the stop button not have any light pattern makes sense to me though - it neither ran to the end of the program nor had an unhandled exception

[laurensvalk]

Agreed. Another way to catch those might be to have a program start indication, which may also cover #139 (comment).

[laurensvalk]

I would like to have an indication when the user program ends that lets the user know why it ended:

1. Reached end of program (includes raising SystemExit - or maybe all BaseException like KeyboardInterrupt too?)
2. Program aborted due to unhandled exception.
3. Stop button pressed (not sure if this one deserves a separate indication - or no indication?)

This is especially helpful when running a program while not connected to a computer.

4. Similarly, something to indicate that there isn't any program yet.

[afarago]

I might be too out of box here, yet how about an audio notification?
(Good old PC beep codes ;)

Normal end - none
Exception - G/4,E/4,C/2
Stop pressed - G/8
No program in slot - C/2

For example....

[laurensvalk]

Yes, especially on hubs with a broken screen (I've been making progress on the NXT last night 🤫).

Short sound patterns might be easier than tones, and not too many different ones.

It would be nice to hear slot switches too.

To get started, I would suggest to take the python program that emulates the spike menu from the issue about the spike UI. Add some sounds there to see what sounds right before doing it in the firmware.

Feel free to discuss in a new issue :)