HubCentral / Portview based on AppData and StartUserProgram slot parameter in protocol v1.4.0
Implementation and first merge of HubCentral/PortView feature based on the discussion https://github.com/pybricks/support/issues/1708
Slightly modified hubcentral protocol, included _builtin_port_view.py as well for reference.
Issues/topics to clarify:
NOW:
- Sporadic error especially with special characters (unicode issue?) e.g. with char “°”
- Is the current flow acceptable to activate the portview app (via showDialog / hideDialog)? or should we need more secure and synchronous flow? If yes, not clear how to solve.
- This layout supports 6-port hub only; guess I need to check the essential and the technic hub as well. (No move hub support, right?). Any best practice to easily detect hub type?
NEXT:
- Is using slot_id = 129 OK? Which command to use: StartUserProgram = 1 OR StartRepl = 2 (PBIO_PYBRICKS_COMMAND_START_BUILTIN_PROGRAM). Are there any differences or builtin/repl will get deprecated?
LATER:
- icons and visual design following up on Laurens's virtual hub design
- handle unknown device icons/visuals
- mode change (app->hub)
Thanks for taking the time to rework this with the firmware changes!
Sporadic error especially with special characters
The hub port view script is still breaking down messages to 19 byte chunks, so we could be seeing this?
We could look into using the negotiated MTU on the firmware side so we can send whole messages in one go.
I was initially thinking we could send pre-formatted strings, in order to limit the device specific stuff we need to add to Pybricks Code. But if make device specific visualizations anyway, we can look at packing a single port value into a 19 byte message, which should be feasible.
Is using slot_id = 129 OK?
Yes.
Which command to use: StartUserProgram = 1 OR StartRepl = 2
StartUserProgram with new one byte payload
Are there any differences or builtin/repl will get deprecated?
The old command PBIO_PYBRICKS_COMMAND_START_REPL is internally mapped to StartUserProgram(128) for backwards compatibility with older versions of Pybricks Code.
No move hub support, right?
Correct. There is a new feature flag PBIO_PYBRICKS_FEATURE_FLAG_BUILTIN_USER_PROGRAM_PORT_VIEW in the hub features you can check, instead of coding any hub checks in Pybricks Code. Compare with the existing and unchanged PBIO_PYBRICKS_FEATURE_FLAG_BUILTIN_USER_PROGRAM_REPL feature flag to see how this is used to disable the REPL button on Move Hub.
This layout supports 6-port hub only; guess I need to check the essential and the technic hub as well. (...) Any best practice to easily detect hub type?
Since we're providing everything for the device visuals from the Python script, we can include hub type also.
I think we'll want to selectively include some of these modules in the firmware if we end up making fancy things for Prime Hub (Like remote-control interfaces), so that we don't increase the other firmware sizes unnecessarily.
- Sporadic error fixed - thanks for the hint
- Visuals Improved.
- PBIO_PYBRICKS_FEATURE_FLAG_BUILTIN_USER_PROGRAM_PORT_VIEW is in the firmware code, I don't see how I could leverage it from user-python or from pybricks-code. Btw: at the moment I can use the version (from usys import version) or the number of ports returned in pybricks-code.
PBIO_PYBRICKS_FEATURE_FLAG_BUILTIN_USER_PROGRAM_PORT_VIEW is in the firmware code, I don't see how I could leverage it from user-python or from pybricks-code.
See how pbio_pybricks_feature_flags_t with PBIO_PYBRICKS_FEATURE_FLAG_BUILTIN_USER_PROGRAM_REPL is equivalent to HubCapabilityFlag with HasRepl : https://github.com/pybricks/pybricks-code/blob/482a5547790d94a4c2a89bc00e11609bdc4dd7a0/src/ble-pybricks-service/protocol.ts#L267
This is used to enable the button in ReplButton.tsx.
-
Added the feature flag and if does not exist, reverted back to the old info popup.
-
Changed color display to hsv (hsb) as you suggested
-
There is a number of devices and hubs to be rendered, yet as a next step, please help me understand what are the MVP next steps to get this PR merged. I am quite enthusiastic to make it happen:
- is the current (portview<->pybricks-code) protocol acceptable or do you want to change it? I would be OK with the current protocol:
[Port.<A-F>|battery|buttons|imu]\t[port type|field0]\t[fields separated by tab]- pro: the current protocol is versatile and quite fault tolerant (display as is, if needed consume some fields), still if that fails there is a graceful degradation (e.g. shaft is not moving, color is not showing up)
- con: slightly more data over appdata channel
- if yes, would it be possible to finalize --> I would adopt and implement this on both sides
There is a number of devices and hubs to be rendered, yet as a next step, please help me understand what are the MVP next steps to get this PR merged. I am quite enthusiastic to make it happen:
Thank you!
I am currently working on releasing the current state of things as beta with all the recent firmware updates. The most relevant firmware issues have now been fixed. Then I plan to turn my attention to the block coding backlog and Pybricks Code.
please help me understand what are the MVP next steps
- I think this is a good time to review exactly how we want this to look. I think this is a good start. We can make some visual mockups on how we might want to scale the device images and display and lay out the measurements.
- We can work on the encoding protocol, see https://github.com/pybricks/support/issues/1859 opened a few days ago.
- Finalize and document the hub protocols added so far and suggest any changes if we expect to need any.
- Update Pybricks Code to work with the new protocol, without port view just yet. So all the new commands, feature flags, etc. Then everything will be prepared for this new port view feature.
To make it easier on me to review, could we first split out the Pybricks Profile 1.4 changes to a separate pull request?
A naive remark and may be out of order.
- I think I miss the Technic-move motor.
- How does the Color-Distance sensor show the distance. All I see is it is getting mode = 0.
Tell me to shut-up if this is all irrelevant.
What a great project this is!
Bert
All ideas are welcome here!
- Are you referring to Medium Linear Motor ?
- You simply change modes...
This is a sneak peak of my work-in-progress version.
Keep the excitement coming, we have some UI design and review work to be done...
All ideas are welcome here!
- Are you referring to Medium Linear Motor ?
I was referring to the Technic move hub built-in motors:
| ID (hex) | Description |
|---|---|
| 86 (0x56) | Technic Move hub built-in drive motor |
| 87 (0x57) | Technic Move hub built-in steering motor |
- You simply change modes...
I miss something, then. I plug the sensor into the hub, how to change the mode? [EDIT] You just showed me, sorry a UI thing I did not see in the python.
snipped the picture
Keep the excitement coming, we have some UI design and review work to be done...
Yes, please.
Bert
Hi Attila,
It took me quite some fiddling to see why CityHub and TechnicHub only showed pup devices. In line 29 in _builtin_port_view.py
hub = ThisHub
the parens are missing, should be:
hub = ThisHub()
With the result that hub.battery.voltage() is refused. And other things.
Bert
I think the focus for now is mostly on the PrimeHub, but all hubs except the move hub should be supported. Het program is already too large for the movehub.
Small addition: This snippet near line 260:
def battery_task():
if not hub.battery: return
count = 0
while True:
count += 1
if count % 100:
yield None
else:
# skip cc 10 seconds before sending an update
percentage = round(min(100,(hub.battery.voltage()-6000)/(8300-6000)*100))
voltage = hub.battery.voltage()
status = hub.charger.status()
data = f"pct={percentage}%\tv={voltage}mV\ts={status}"
yield f"battery\t{data}"
Not all hubs that support hub.battery also have a charger.
But status = hub.charger.status() prevents to report all available battery data.
To make it easier on me to review, could we first split out the Pybricks Profile 1.4 changes to a separate pull request?
Hi @dlech , would the granularity of the #2317 be OK for a review size and for us to move forward? I also corrected some tests and added new ones for the new protocol functions.