pybricks / support

Pybricks support and general discussion
MIT License
109 stars 7 forks source link

[Feature] Toggle Bluetooth with Bluetooth button. #1615

Closed laurensvalk closed 5 months ago

laurensvalk commented 7 months ago

We are planning to use Pybricks for the WRO RoboMission.

Having bluetooth enabled permanently is problematic for competition purposes. The rules (Section 5.11 in https://wro-association.org/wp-content/uploads/WRO-2023-RoboMission-General-Rules.pdf) cause some inconveniences and there is the general problem that a connection is easy to hijack which can be used to ruin a run of a competitor. While that is forbidden it would be very difficult to pinpoint an attacker.

The cleanest way to resolve this would be to disable bluetooth (at least temporarily). It seems to me that this is one of the few things which are actually better with the lego firmeware which only enables bluetooth after the dedicated hardware button is pressed.

Originally posted by @DrTom in https://github.com/pybricks/support/discussions/1089

laurensvalk commented 7 months ago

Also requested by @Kermit647 @afarago @ttn-c4plus @nkarstens and others. See also https://github.com/pybricks/pybricks-micropython/pull/195.

laurensvalk commented 7 months ago

Some ideas for the user experience:


Suggestions for color indications:

** Thoughts? I was thinking a red color is more explicit than off, more easily communicated to judges, preventing cheating by using an older firmware. But if you say light off is better, we could do that.


Scope: only for SPIKE Prime and Robot Inventor? Is this also needed for SPIKE Essential?


Alternate ideas, Bluetooth on for coding, but no RC

If I'm understanding it right, the above has no technical utility, but it's to satisfy robot competition rules.

If that's the case, would it make sense to allow coding, but just disable all in-program Bluetooth features like remote control and broadcasting/observing?

ggramlich commented 7 months ago

As @laurensvalk asked for feedback:

This video "WRO Robo-Check" is provided to inform the German judges. https://www.youtube.com/watch?v=xQGuZWgKVKo If you want to optimize for WRO compatibility, it would be best to resemble this behavior:

I do understand, that in a home setting, turning bluetooth on automatically is convenient, but actually as you still have to press the connect button in the Pybricks software on the computer and could show some message there (as you already do right now, if no device is found / the user cancels). The message could ask you to switch on the robot and long-press the bluetooth button (and still provide help to install the firmware). It is not a big deal to force the user to do another button press on the robot. Every user switching from the Lego software is already used to that. And once you are connected, you stay connected.

Alternate ideas, Bluetooth on for coding, but no RC

The main reason for this rule is NOT that they want to prevent remote controlling the robot, but to prevent that you might switch the program. There is a randomization for each run in the competition and for some objects you don't know where they are placed. The program is supposed to handle all of these situations by recognizing them (or possibly have some mechanical measures that work for all cases). There is a countdown and the robot has to be placed on the parking area before the countdown runs out with the program for the next run on it. This is where the robots should be turned off or have Bluetooth disabled. After the robot check, the actual randomisation is revealed and this is the time where a cheating team might change the program on their robot (or a malicious team might delete the program on your robot).

So "Bluetooth on for coding, but no RC" is not an option.

Scope: only for SPIKE Prime and Robot Inventor? Is this also needed for SPIKE Essential?

I personally would not care about Spike Essential, I have never seen them in a competition.

Bluetooth can only be toggled while the robot is not connected to the computer. (Open question. Thoughts? Or should pressing it forcefully disconnect?)

That would be fine, maybe disconnecting from the computer should simply lead to the state "Bluetooth off".

nkarstens commented 7 months ago

I think the best experience will be to save the setting to persistent storage. The kids can program it for the months leading to the competition without forgetting to enable it, and then they can take it to competitions without worrying about forgetting to disable it.

My preference would be red = off and blue = on. With this approach it will be hardest to make a mistake and not notice that you are in the wrong state. I'd be fine with no light = off as well, if others feel strongly.

afarago commented 7 months ago

In the meantime I have been to some WRO competitions either as a coach or in some countries I have the honor to be head judging the WRO competition.

As for the SPIKE/RI sets Bluetooth is always on even with the stock firmware, therefore turning bluetooth off is not really an immediate need anymore in my opinion.

There are a few points for best competition experience left still, some connected with BT.

laurensvalk commented 7 months ago

Thanks everyone for the input! Combing all that with the base suggestion, it seems like the following gets quite close to most what you'd need:

Regarding suggestions such as long-press / additional BT authentication step / enabling discovery, I think we would rather avoid those, especially since you could now achieve off-on-boot as above. Already, discoverability is disabled once a connection is established.

I think bluetooth on/off with a simple button/light is easier to understand for kids.

I'm not a big fan of the separate discoverability button press in the official app. In my opinion that's too many different clicks, mouse moves, and buttons on the hub just to establish a connection.

m7thon commented 7 months ago
  • Bluetooth is on by default, same as now (no breaking change).
  • Toggle Bluetooth on/off with button.
  • Bluetooth light: blue is on, red is off. We can reserve other colors for other future behaviors, if we ever want them (e.g green = coding but no RC).
  • Make it persistent after reboot. So you can achieve off-on-boot, for judge review. Judges also see the visual indicator.
  • Spike Prime / Robot Inventor only.

I used the branch from Nate Karstens implementing exactly this behavior with two WRO teams in the preparation and the event, and this was easy and intuitive for the kids to use (and I believe this fulfills the WRO rules).

nkarstens commented 7 months ago

To be clear, pybricks/pybricks-micropython#195 does not support persistent settings yet.

@laurensvalk in the discussion on the PR you mentioned that there were still some things that you were figuring out regarding persistent settings. Do you have any more information on that?

laurensvalk commented 7 months ago

there were still some things that you were figuring out regarding persistent settings. Do you have any more information on that?

I've opened an issue for it in https://github.com/pybricks/support/issues/1622

afarago commented 7 months ago

Could we consider having 3 BT modes?

  1. BT on open (current)
  2. BT off
  3. BT on secure - only connect to last connected MAC (after mode 1) - needs persistent storage for MAC #1622

Though kids are nice it would be a disaster if during the competition anyone would connect to your hub when doing the programming / testing part.

(You know the analogy - in the IoT abbreviation the letter "s" stands for security)

laurensvalk commented 6 months ago

https://github.com/pybricks/pybricks-micropython/pull/247 implements the following. See the rationale in that PR for some of the technical details.

This could serve as a practical starting point for (teams in need) now, but I'm open to reconsidering the UI on Prime Hub now that we have this, possibly deviating from the other hubs. Maybe it's the Bluetooth light that should blink, for example.

The setting is not persistent for now, but this should probably not hold back the rest of it.

image

https://github.com/pybricks/support/assets/12326241/e6ef0205-2ebc-4d08-923b-b4e7a826fdc8

laurensvalk commented 6 months ago

To try this out, download the firmware file and install it on your hub with these instructions.

If you try this out, please let us know how it goes!

BertLindeman commented 6 months ago

Probably expected message:

The connected hub uses a newer Pybricks communication profile version. Some features may not work correctly.

The hub has v1.4.0 while Pybricks Beta supports v1.3.0.

Downgrade the hub firmware or upgrade Pybricks Beta to avoid problems.
laurensvalk commented 6 months ago

Thanks @BertLindeman. Yes, that is the expected message for now. It won't make a difference in this case.

If/when this is released, the Pybricks Code editor will be updated to match.

BertLindeman commented 6 months ago

A situation I can not reproduce.

After running a program on a spike prime with this firmware, I obviously tried to press the BT button where we would not expect a change. That went very good. BT-button press if connected or program running does nothing. Once (only once :-( ) trying to get the hub connected to beta.pybricks.com did not work: BT-button blue but not found by the app. Needed to power-off/on to get connected.

laurensvalk commented 6 months ago

Maybe a program was running, so it's not available for connecting?

This is one of the reasons I'm considering a UI overhaul. With the Bluetooth light enabled, there are three different things to look at, which is confusing.

BertLindeman commented 6 months ago

Maybe a program was running, so it's not available for connecting?

This is one of the reasons I'm considering a UI overhaul. With the Bluetooth light enabled, there are three different things to look at, which is confusing.

The center button was blinking. Ho, got it..... I think. The hub was still on USB after the firmware install. And that makes the difference. Scenario:

  1. hub on USB loading
  2. connect hub to app
  3. run a program
  4. stop the program
  5. disconnect BT
  6. press BT-button (now red) and see no hub in scanlist on app
  7. press BT-button (now blue) and center button blinking and see no hub in scanlist on app

In the meantime the hub timeout and that "fixes" it.

The situation only occurs if a program has been run in this scenario. power-on / BT-off / BT-on even on USB is fine.

The program is this case ```python """ program to show the battery voltage and current consumed over time Simplified arithmetic so this can also run on the movehub """ from pybricks import version from pybricks.tools import wait from pybricks.tools import StopWatch from pybricks.parameters import Color print(f'version {version}') DEBUG = False # DEBUG = True INTERVAL_MS = 30 * 1000 # 30 seconds in mSec BAT_HI = 9000 # mVolt BAT_MED = 6500 # mVolt BAT_LOW = 5500 # mVolt BAT_OUT = 4800 # mVolt HEADER = '\033[95m' OKBLUE = '\033[94m' OKCYAN = '\033[96m' OKGREEN = '\033[92m' WARNING = '\033[93m' FAIL = '\033[91m' ENDC = '\033[0m' BOLD = '\033[1m' UNDERLINE = '\033[4m' hw_type = version[0] # import and define the HUB object if str(hw_type) == "technichub": from pybricks.hubs import TechnicHub HUB = TechnicHub() elif hw_type == "movehub": from pybricks.hubs import MoveHub HUB = MoveHub() elif hw_type == "inventorhub": from pybricks.hubs import InventorHub HUB = InventorHub() HUB.display.off() elif hw_type == "primehub": from pybricks.hubs import PrimeHub HUB = PrimeHub() HUB.display.off() elif hw_type == "cityhub": from pybricks.hubs import CityHub HUB = CityHub() # HUB.display.off() ## No display on cityhub else: print("Unknown HUB:", hw_type) # Movehub does not have math functions, so no float() just shift the decimal point in def format_ms_to_time(ms): # Constants representing the number of milliseconds in one second, minute, and hour ms_per_second = 1000 ms_per_minute = 60 * ms_per_second ms_per_hour = 60 * ms_per_minute # Calculate the hours, minutes, and seconds hours = ms // ms_per_hour ms %= ms_per_hour minutes = ms // ms_per_minute ms %= ms_per_minute seconds = ms // ms_per_second ms %= ms_per_second return hours, minutes, seconds, ms def format_msec_as_time(clock): # use decimal math not float, so movehub can do it too. hour, minute, sec, ms = format_ms_to_time(clock) return f'{hour:>2}:{minute:>02}:{sec:02}.{ms:03} - ' def print_time_and_voltage(): if BAT_HI > HUB.battery.voltage() > BAT_MED: textcolor = OKGREEN + BOLD elif BAT_MED > HUB.battery.voltage() > BAT_LOW: textcolor = WARNING + BOLD HUB.light.on(Color.YELLOW) else: textcolor = FAIL + BOLD HUB.light.on(Color.RED) print(format_msec_as_time(watch.time()), end=" ") cur_voltage = format_like_decimal(HUB.battery.voltage()) print(f"{hw_type:<12} {textcolor}voltage: {cur_voltage:>6} V{ENDC}", end=" ") def print_amperes(): print(f"{hw_type:<12} current: {format_like_decimal(HUB.battery.current()):>6} A") def format_like_decimal(mynum): sign = '-' if mynum < 0 else '' num_str = str(abs(mynum)) num_len = len(num_str) result = '' if num_len == 1: result = sign + '0.00' + num_str elif num_len == 2: result = sign + '0.0' + num_str elif num_len == 3: result = sign + '0.' + num_str else: for i in range(num_len): if i == num_len - 3: # was 4 result += '.' result += num_str[i] return result # tests if DEBUG: print(format_like_decimal(-1000)) print(format_like_decimal(-100)) print(format_like_decimal(-10)) print(format_like_decimal(-1)) print(format_like_decimal(0)) print(format_like_decimal(1)) print(format_like_decimal(10)) print(format_like_decimal(999)) print(format_like_decimal(1000)) print(format_like_decimal(10000)) print(format_like_decimal(100000)) watch = StopWatch() watch.reset() while True: # determine the time it took to deliver our payload and subtract that from waittime start_time = watch.time() # get the current time print_time_and_voltage() print_amperes() end_time = watch.time() # get the time after the action processing_time = end_time - start_time # calculate the time taken for the action # wait for the remaining time wait(max(INTERVAL_MS - processing_time, 0)) ```
laurensvalk commented 6 months ago

Thank you! I can't seem to reproduce that, though. Does it still happen if you run this program?

In case it makes a difference, did you stop the program via the app or the hub button?

image

EDIT: I think I was able to get it into that state once, though not with that method. I'll have a closer look.

BertLindeman commented 6 months ago

Added the program above in case..

I stopped and disconnected from the app only.

Your small block program does not trigger the situation. I am not sure if it is important that the hub was powered ON of OFF at USB plugin time.

laurensvalk commented 6 months ago

Pressing it very quickly 10 (or some even amount) times is sufficient to reproduce the issue. USB and program do not seem to matter. I'll fix it tonight or tomorrow.

Meanwhile, I'll fold the intermediate posts to keep it easier to follow this thread. Thanks!

nkarstens commented 6 months ago

@laurensvalk I don't have time to do a deep-dive into this at the moment, but in case it saves you time, this sounds like an issue I ran into in my original implementation where the Bluetooth button needed to debounce (which is why I used separate states for PBIO_PYBRICKS_STATUS_BLUETOOTH_BUTTON_PRESSED and PBIO_PYBRICKS_STATUS_BLUETOOTH_SHUTDOWN.

laurensvalk commented 6 months ago

Thank you. Intermediate states are generally managed quite conveniently by the protothreads. The PYBRICKS_STATUS flags are mainly intended to communicate the state to the "outside".

Technically, it doesn't even have to be a status. Maybe it can be more like bluetooth_is_connected() and just be a function call.

Usually directly over Bluetooth (e.g. to report low battery), but in this case they're also used in some higher level modules like the Remote class to raise an appropriate exception when these classes are used when Bluetooth is disabled.

I'll rework the bluetooth system protothread and report back. Next iteration will be posted tomorrow :)

Debenben commented 6 months ago

Sorry for the silly and curious remark from someone who has never participated in any WRO competition: If you wanted to cheat, why not use your own pybricks fork, set the blutooth button to whatever color you like and only use the bluetooth chip to passively listen to incoming broadcasts (maybe not using lego company identifier but something less suspicious). Not sure if this would even be forbidden since the rules talk about "connection".

laurensvalk commented 6 months ago

It's probably not so much a technical issue, but more about ensuring we provide an easy way for the official Pybricks version to comply with existing competition rules. Judges are volunteers also, so it's best to keep things as simple as possible.

So it has to be easy to use, but also easy for the students to know that they've done it the right way. So they can confidently present their hard work to the jury.

I would rather have the jury ask the teams about their innovative solutions, instead of distracting them with lights that may be perceived to blink the wrong way :)

laurensvalk commented 5 months ago

@BertLindeman - I've updated the build in case you want to try it.

BertLindeman commented 5 months ago

@BertLindeman - I've updated the build in case you want to try it.

Was expecting a link to the build, but I assume you mean build 3380. If I hurry I can just do it before I go assist on a course "HELP, I have a smartphone" 😄

BertLindeman commented 5 months ago

Did a quick test and did not see the previous problem.

Thought I saw something strange but the same test on RobotInventor at 3.5.0 does the same.

For the curious - run a program (for me: tools try_movehub_on_prime.py) - normal stop from the app and disconnect from the app - stop the hub so the program gets saved - without (re)connecting to the app start the program (button press) - within seconds the hub shuts down on it's own; Battery is at voltage: 6.955 V As said both this build 3380 prime and 3.5.0 RobotInvenotor do it. Program is garbage fiddling to get movehub randint with a lot of printing. Will try later to get a smaller program. But that is another issue then.
laurensvalk commented 5 months ago

Work in progress demo with persistency after reboot:

https://github.com/pybricks/support/assets/12326241/9b181f07-95ba-4d68-91cc-1aab46ac6f57

laurensvalk commented 5 months ago
  • Bluetooth is on by default, same as now (no breaking change).
  • Toggle Bluetooth on/off with button.
  • Bluetooth light: blue is on, red is off. We can reserve other colors for other future behaviors, if we ever want them (e.g green = coding but no RC).
  • Make it persistent after reboot. So you can achieve off-on-boot, for judge review. Judges also see the visual indicator.
  • Spike Prime / Robot Inventor only.

This has now been implemented! Thanks everyone for pushing for this.

If you want to try it out before we release a new beta, just follow these instructions. That page includes a link to the latest build.

afarago commented 5 months ago

Is this normal that every time I toggle the bluetooth off and on I see a "new" device witha seemingly random MAC address?

At the moment as due to browser bluetooth API limitation there is no autoconnect - it does not make a difference, yet seems odd.

image

dlech commented 5 months ago

Yes, we had to do that to work around issues with OS Bluetooth stacks caching the GATT attributes that change when the firmware is changed. Things just work better when it is acting like it is connecting to a new device every time.

afarago commented 4 months ago

Yes, we had to do that to work around issues with OS Bluetooth stacks caching the GATT attributes that change when the firmware is changed. Things just work better when it is acting like it is connecting to a new device every time.

Is there any chance to reconsider this?

Checking the web bluetooth developments we already have several routes to reconnect to a previously allowed device. This would significantly improve the user experience of pybricks code IDE.

I have played around with the code and checked this approach using navigator.bluetooth.getDevices() - it seemed quite flawless from IDE perspective yielding to a potentially small amount of changes.

laurensvalk commented 4 months ago

Yes, we had to do that to work around issues with OS Bluetooth stacks caching the GATT attributes that change when the firmware is changed. Things just work better when it is acting like it is connecting to a new device every time.

Is there any chance to reconsider this?

Maybe when https://github.com/pybricks/pybricks-micropython/pull/246 is a bit more polished, Bluetooth is probably as complete as it gets. So if we're happy with all the Bluetooth features and agree not to change them anymore, I suppose we could consider that again at some point.

dlech commented 4 months ago

This issue is closed, so I would highly recommend to start a new issue so that this discussion does not get lost and forgotten.

zoligyenge commented 2 months ago

Hello the latest page link appears broken. Could you please post the new link to the latest builds? Thanks

If you want to try it out before we release a new beta, just follow these instructions. That page includes a link to the latest build.

dlech commented 2 months ago

https://pybricks.com/learn/getting-started/install-pybricks/#trying-the-nightly-version-optional