Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Feature] Hub display dialog / bluetooth advertisement on hub and connected PUP devices status #1708

Open
afarago opened this issue Jul 10, 2024 · 19 comments
Labels
enhancement New feature or request triage Issues that have not been triaged yet

Comments

@afarago
Copy link

afarago commented Jul 10, 2024

Is your feature request related to a problem? Please describe.
When connecting and experimenting with a hub, the current IDE does not help the students to sufficiently explore their hub and the connected perioferials.

Describe the solution you'd like
I would love to have a status dialog displaying the hub status and connected periferials.
As a plus I could imagine a "heart program" simple control mechanism or even an EV3 "port view"

Describe alternatives you've considered
The dialog I would welcome could display

  • connected hub, name, battery, BT signal strength
  • connected devices and their values (sensor / motor)
  • maybe SPIKE hub led matrix

This would require a constant BT advertisement, that is currently containing only a few values on the hub, nothing on the PUP devices.
Similar one is available by the default LEGO implementation over the (tbc) 0000fd02-0000-1000-8000-00805f9b34fb service/characteristic.

Additional context

my example (IDE only) mock

@afarago afarago added enhancement New feature or request triage Issues that have not been triaged yet labels Jul 10, 2024
@laurensvalk
Copy link
Member

laurensvalk commented Jul 11, 2024

I would love to have a status dialog displaying the hub status and connected periferials.
As a plus I could imagine a "heart program" simple control mechanism or even an EV3 "port view"

Fully agreed! 👍

UX thoughts
This mainly boils down to whether we want to have Port View while the program is running too, or just when idle.

Allowing it only while no program is running is probably the simplest, as discussed below. This is also the best for the user, I think. Live during a program introduces unwanted lag.

UI ideas

  • Instead of adding a new button at the top, I think we could show your suggested dialog when the user clicks on the connected hub at the bottom right. The small status view we have currently could go away.
  • Instead of abstract icons, we could also consider showing something a bit like this. In particular, it would be nice to intuitively visualize the 0 point of the motor instead of showing just a degree value.

Implementation idea 1: Builtin port view program
There is potentially a comparatively easy way of doing this without inventing new protocols. Opening the dialog could trigger a builtin program on the hub to run, just like triggering the REPL. Closing the dialog or pressing the hub button stops it.

That builtin program could be implemented entirely in Python. It would send all the sensor values to the PC. Somewhat similar to stdout, though perhaps using pybricks/pybricks-micropython#246 so the user doesn't see it in the terminal output window.

These sensor values could be formatted strings like -379° that can be displayed as-is, avoiding the need for protocols or logic that could break between updates on the firmware or IDE side. Or perhaps "A,132,-379°" to indicate the port, device/icon ID and the value to display.

Likewise, pybricks/pybricks-micropython#246 can also be used the other way to select a sensor mode if we want to support that. For the color sensor specifically, we can already show reflection and color and a live colored box representing the HSV value without changing any modes. Modes would just be needed for ambient sensing.

And since this approach uses a builtin program, it can also work offline to display values on the hub display like the heart program you suggested. Maybe a particular button combination (left + right) could start it, or we could expand the the hub UI a bit.

@afarago
Copy link
Author

afarago commented Jul 11, 2024

First of all, thanks for the prompt ideation, you are awesome.

Let's focus first on getting the data out of the hub, then we can focus on the UX aspects.
Agreed - it makes much more sense to only make this data available when the hub is connected and idle. This renders any custom BLE unnecessary - agreed.
It might not make sense to have this always available (LEGO SPIKE App always shows sensors even in the code editor window) - I do not think this is useful.

Following up on this having a piece code available on the brick sending the data is a valid concept.

As I understand we can evaluate two alternatives

  1. have a port view sender module part of the firmware and connect to this
  2. when starting the port view window a custom code is downloaded via ~REPL

Any preferences?
By myself I think I can progress by creating this python code to report this data.
Can I get some pointers how I could accomplish from the react app a REPL start and download (or any alternatives)?

@laurensvalk
Copy link
Member

laurensvalk commented Jul 11, 2024

when starting the port view window a custom code is downloaded via ~REPL

Just to clarify, I meant that the program can be a Python module that we include in the firmware, so there isn't any download time. What I mean with "start like the REPL", is that we could have a similar command to starting this builtin Port View program. You can have a look around Pybricks Code to see how the REPL is currently activated. (Unlike most MicroPython variants where the REPL is something special, a bit like the OS, we just treat the REPL like any other user program. It just so happens to use stdin/out a lot, but otherwise isn't special.)

We could start by just writing and starting it like normal Python code. We could also start by printing the result to standard out, and start thinking about merging pybricks/pybricks-micropython#246

@afarago
Copy link
Author

afarago commented Jul 13, 2024

I am coming up with the hub side program.
Is this something that we could think about? I am not sure about the firmware constraints, therefore I was rather thinking on the REPL on demand download.

This is the sample output that I would channel to the pybricks-code IDE.

H,Luna,3.4.0; Pybricks MicroPython v1.20.0-23-g6c633a8dd on 2024-04-11,SPIKE Prime Hub with STM32F413VG,7430
H:BAT,8393,114,0
H:BUT,
H:IMU,0,1
P,A=63,B=61,C=48,D=x,E=63,F=x
P:A,63,0
P:B,61,-1
P:E,63,0
P:E,63,1
P:E,63,48
P:E,63,53
P:E,63,1
P:E,63,0
H:BUT,B
H:IMU,-1,0
P,A=63,B=x,C=48,D=x,E=63,F=x
H:IMU,-1,1
H:IMU,-1,0
H:BUT,C

Any hints how I can channel the terminal output to the dialog?
I will experiment around React, yet any hints are more than welcome.

This is the code: https://gist.github.com/afarago/350a931992e05c423860f4b25ea5c6a2

@BertLindeman
Copy link

Hi Attila,

Not really in line with your question, but pylint found two syntax errors in your program:
Both times quotes in an f-string.
On line 54:

StatusReporter((lambda : f"H:BUT,{"+".join(sorted(str(b).replace("Button.","")[0] for b in hub.buttons.pressed()))}"), None, 100),

Might be (6 double quotes replaced by single ones):

StatusReporter((lambda : f"H:BUT,{'+'.join(sorted(str(b).replace('Button.','')[0] for b in hub.buttons.pressed()))}"), None, 100),

And similar on line 80:

retval.append(f"{portid}={self.deviceids[idx] if self.deviceids[idx] else "x"}")

Might be:

retval.append(f"{portid}={self.deviceids[idx] if self.deviceids[idx] else 'x'}")

I also ran your program on Technic hub and city hub.
Works OK.

Interesting program, thank you,

Bert

@afarago
Copy link
Author

afarago commented Jul 14, 2024

Hi Attila,

Not really in line with your question, but pylint found two syntax errors in your program: Both times quotes in an f-string. On line 54:

StatusReporter((lambda : f"H:BUT,{"+".join(sorted(str(b).replace("Button.","")[0] for b in hub.buttons.pressed()))}"), None, 100),

Might be (6 double quotes replaced by single ones):

StatusReporter((lambda : f"H:BUT,{'+'.join(sorted(str(b).replace('Button.','')[0] for b in hub.buttons.pressed()))}"), None, 100),

And similar on line 80:

retval.append(f"{portid}={self.deviceids[idx] if self.deviceids[idx] else "x"}")

Might be:

retval.append(f"{portid}={self.deviceids[idx] if self.deviceids[idx] else 'x'}")

I also ran your program on Technic hub and city hub. Works OK.

Interesting program, thank you,

Bert

Thanks! Stupid question - how do you toolchain pylint?
I am using the code.pybricks.com now, that is awesome, yet pylint and auto format is not av.

@dlech
Copy link
Member

dlech commented Jul 14, 2024

Any hints how I can channel the terminal output to the dialog?

We will either need a new event type or status bit to indicate that this special program is running and to be able to redirect the standard output elsewhere.

@BertLindeman
Copy link

Thanks! Stupid question - how do you toolchain pylint? I am using the code.pybricks.com now, that is awesome, yet pylint and auto format is not av.

I have my sources on Linux Mint and copy them into pybricks.
So I have pylint on linux.

@laurensvalk
Copy link
Member

Thanks for working on this.

Any hints how I can channel the terminal output to the dialog?

We will either need a new event type or status bit to indicate that this special program is running and to be able to redirect the standard output elsewhere.

I think this could be covered by the suggested approach above. Only this program would be started by the dialog. And it need not use standard output.

Of course you're welcome to work on it as you like, but it may be a bit soon to commit to a particular approach :)

I would like to make some time to clean up pybricks/pybricks-micropython#246 and keep #1708 in mind along the way. See also the recent note there about sending data back to the IDE.

@afarago
Copy link
Author

afarago commented Jul 15, 2024

Here is my first naive and wacky implemetation.
https://github.com/afarago/pybricks-code/tree/experiment-hubcentral

image
And the demo video: https://photos.google.com/share/AF1QipO47B0qloVnoIxXApMJsUyUWYs81w2MKyqVvzJEJNopYzwn7MLgs8BmvyrR1Po62A/photo/AF1QipOcD6qFdfQ7nt0w9yYZjpAzdFLW93JLpzBsn8MT?key=ZTZXaFk4c2pYOGhHY3gzZHEyQlB0RlNLOE1mLVFB

Will keep on experimenting with the HostBuffer approach, currently just hacked and replicated the terminal stream.

@afarago
Copy link
Author

afarago commented Jul 17, 2024

I think I am pretty much done with the base functional implementation.

image

Let me know how we could move on with the next steps.
Also any review comments are welcome, to get code compatible/compliant.

What is definitely left

  • HostBuffer / AppData usage
  • firmware inclusion of the hubcenter code (or similar)
  • IDE to activate the hubcenter mode on the connected brick
  • current implementation follows dark mode, need to make it cross compatible for both modes

@laurensvalk
Copy link
Member

Whoa, that looks amazing! I think it's going to take quite a bit of time before I can get to this, but I'd really like to merge something like this eventually!

@laurensvalk
Copy link
Member

Here's a tangentially related idea that could perhaps be its own topic in the future. I'm just mentioning it now in the context of potentially keeping the communication schemes very generic.

In addition to a port view, we could have a live variable view for the currently running script. Since we already require variables to be declared in the setup section, we could quite easily add > option on their setup block to activate monitoring it. Doing so could generate an async background task that sends that value as app data when it changes.

In the same vein, you can think of a way to highlight the current block or stack.

@laurensvalk
Copy link
Member

FWIW, I am currently working on the AppData feature in pybricks/pybricks-micropython#246, as well as a generalized way to start programs.

Currently we have:

pbio_error_t pbsys_user_program_start_program(void);
pbio_error_t pbsys_user_program_start_repl(void);

These commands will be internally generalized to:

pbio_error_t pbsys_main_program_request_start(pbsys_main_program_type_t type, uint32_t id);

At the protocol level, I would like to propose to rename PBIO_PYBRICKS_COMMAND_START_REPL to PBIO_PYBRICKS_COMMAND_START_BUILTIN_PROGRAM and give it an identifier payload. The payload can be optional for backwards compatibility.

Likewise, PBIO_PYBRICKS_COMMAND_START_USER_PROGRAM can get an optional identifier payload too so we don't have to modify again at some point in the future.

The other option would be to make these new, separate commands.

@afarago
Copy link
Author

afarago commented Jul 23, 2024

Additionally an idea:
If we are able to reliably and easily transmit what is connected to the hub, that could enable the IDE to generate the template with all the peripherials already initalized, e.g motors, color sensor on the righr port both in text and blocks mode.

That would speed up the onboarding process for newbies.

@laurensvalk
Copy link
Member

laurensvalk commented Jul 25, 2024

On an implementation level of the Python side, here is an experiment with device-specific output:

image

The port and type ID are standard and separated by a tab. The remainder is device specific plain-text we can show in Pybricks Code with limited further processing, if any. The device type ID is just so we can show the right picture.

That way, we don't need to re-implement things such as values[0] = forcevalue if forcevalue > 0 else self.device.read(1)[0] # FORCE or TOUCH since the firmware is already doing those things for us. This also ensures we show exactly the same values as users would see in their own code.

Unknown devices can show their default mode data.

The remainder of the code is a small runloop that pushes these generators along when something gets plugged in. I'll add it to the proposal in #254.

@laurensvalk
Copy link
Member

laurensvalk commented Jul 26, 2024

In the sensor UI previewed by Attila above, relevant sensors can have a dropdown (or make the sensor icon clickable) to select a mode. These need not be all 10+ modes, but just the relevant ones in our API:

For example, the boost color and distance sensor might have:

  • Reflected light intensity and color
  • Ambient light intensity
  • Distance

The list of items to populate the dropdown with comes from the hub script as plain text, too. The user selection (index) can be sent to the hub with app data, so it knows which sensor values to send:

For example:

# BOOST Color and Distance Sensor
def update_color_and_distance_sensor(port, type_id, mode):
    sensor = ColorDistanceSensor(port)

    # This data populates a mode dropdown in the IDE.
    modes = [
        "Reflected light intensity and color",
        "Ambient light intensity",
        "Distance"
    ]
    yield "\t".join([str(port)] + modes)

    # Report the sensor data as preformatted text.
    while True:
        if mode == 0:
            hsv = sensor.hsv()
            ref = sensor.reflection()
            data = f"h={hsv.h}°, s={hsv.s}%, v={hsv.v}%, r={ref}%"
        elif mode == 1:
            data = f"a={sensor.ambient()}%"
        else:
            data = f"d={sensor.distance()}%"
        
        yield f"{port}\t{type_id}\t{data}"

So for the IDE, "incoming data" looks as follows:

Port.A  --
Port.B  75      -115°
Port.C  61      h=120°, s=7%, v=17%, r=9%
Port.D  --
Port.E  --
Port.F  --
Port.A  --
Port.B  75      -115°
Port.C  61      h=60°, s=7%, v=17%, r=9%
Port.D  --
Port.E  --
Port.F  --
Port.A  37      modes   Reflected light intensity and color     Ambient light intensity Distance
Port.A  37      h=0°, s=0%, v=0%, r=0%
Port.B  75      -115°
Port.C  61      h=0°, s=0%, v=19%, r=10%
Port.D  --
Port.E  --
Port.F  --

Multiple whole lines (separated at "\n") may be sent in one package, but lines are never broken up. Within each line, the itself of interest are "\t" separated.

In this example, B and C were already plugged in, and then a sensor was plugged into port A.

@laurensvalk
Copy link
Member

Even though this will only run while no other program is running, it would still be useful to design the UI such that it can be viewed alongside the editor.

Typically, you'd want to use port view to find things like color values or threshold, and you want to enter those into your code as you go. It wouldn't be ideal if you had to go back and forth several times for each value.

@afarago
Copy link
Author

afarago commented Jul 27, 2024

Absolutely right point!

I am happy to contribute to the core content, yet the reason for chosing the dialog was that it had less impact on the overall structure of the app, but definitely could make a lot more sense as a side panel.
Either with spltters or even float in.

If @dlech would modify the app frame (splitters/etc) I can come up with a full implementation for the port view if that helps.

(Later on: It has also the potential to have reactive contents, so if it is resizeable It might even have different levels of granularity.)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request triage Issues that have not been triaged yet
Projects
None yet
Development

No branches or pull requests

4 participants