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

Implement working program slots #266

Closed
wants to merge 0 commits into from
Closed

Implement working program slots #266

wants to merge 0 commits into from

Conversation

laurensvalk
Copy link
Member

This implements pybricks/support#139 and pybricks/support#1790. Because much of the work was already done in #264 and #261, this update is quite small and just implements it in sys/storage.c and sys/hmi.c.

This makes it work using only changes on the firmware side. We could make it a lot better by making accompanying changes to Pybricks Code.

Still, this should be a reasonable way to try out the user experience and find the right improvements.

Feel free to give it a try, @afarago , @BertLindeman !


At minimum, we'll also want to do the following:

  • Introduce a status flag such as PBIO_PYBRICKS_STATUS_RECEIVING_PROGRAM to prevent the UI interactions like a slot change while a big program is being loaded. This isn't done yet, so changing slot while loading lead to failure.
  • Inform Pybricks Code about the maximum size for the current slot, or all slots. Perhaps also include the slot in the event status report (slot is not necessarily the same as program ID). Right now, Pybricks Code won't know that storage is full until it is actually full.

We can probably rethink the logic for downloading programs more generally. At the moment, Pybricks Code sets the program size to 0 (to prevent it from being run), then uploads the program, then downloads the size. This is no longer sufficient, hence the need for a status. Alternately, maybe this should be a process of some kind so the hub can know about failure (and clear that status). Right now, if uploading stops halfway the hub has no way of knowing since it doesn't know the final file size.

Also, we used to provide access to user code on the REPL by allowing you to import it. This is now extended to providing access to the active slot. Do we really want that though? Maybe REPL should just really be independent. It was already kind of broken due to pybricks/support#745 anyway. To get interaction on a stored user module, users can trigger the KeyboardInterrupt instead, which should be sufficient?

@coveralls
Copy link

coveralls commented Sep 6, 2024

Coverage Status

coverage: 56.024% (-0.02%) from 56.042%
when pulling de84b9b on slots
into aa7eb39 on master.

@BertLindeman
Copy link
Contributor

Feel free to give it a try, @afarago , @BertLindeman !

Yes!.
I probably miss the knowledge of how to get this working.
Technic hub did not show any problem.

On the spike prime, however:
I assumed beta.pybricks.com and pybricksdev would write the program to slot zero (or more advanced to the "current" slot). The connect is OK, but after the start of the program
But what I see is the color sensor that happens to be attached goes off and the hub powers-off.
Trying to run the program by button pressed shows a part of the standard display animation followed by poweroff.
Where is my mistake?

test program:

from pybricks import version

print(f'version {version}')

pybricksdev with debug:

pybricksdev -d run ble do_nothing.py

2024-09-06 20:52:19,658: DEBUG: asyncio: Using proactor: IocpProactor
Searching for any hub with Pybricks service...
2024-09-06 20:52:20,574: DEBUG: bleak.backends.winrt.scanner: Received 0C:74:E6:B4:8B:B0: .
2024-09-06 20:52:20,576: DEBUG: bleak.backends.winrt.scanner: Received 0C:74:E6:B4:8B:B0: spike 3540.
2024-09-06 20:52:20,579: DEBUG: bleak.backends.winrt.scanner: 1 devices found. Watcher status: <BluetoothLEAdvertisementWatcherStatus.STOPPED: 3>.
2024-09-06 20:52:20,580: INFO: pybricksdev.connections.pybricks: Connecting to spike 3540
2024-09-06 20:52:20,587: DEBUG: bleak.backends.winrt.client: Connecting to BLE device @ 0C:74:E6:B4:8B:B0
2024-09-06 20:52:20,638: DEBUG: bleak.backends.winrt.client: getting services (service_cache_mode=None, cache_mode=None)...
2024-09-06 20:52:20,882: DEBUG: bleak.backends.winrt.client: session_status_changed_event_handler: id: BluetoothLE#BluetoothLE60:e3:2b:4a:9e:7f-0c:74:e6:b4:8b:b0, error: <BluetoothError.SUCCESS: 0>, status: <GattSessionStatus.ACTIVE: 1>
2024-09-06 20:52:20,994: DEBUG: bleak.backends.winrt.client: max_pdu_size_changed_handler: 527
2024-09-06 20:52:21,853: DEBUG: bleak.backends.winrt.client: 0C:74:E6:B4:8B:B0: services changed
2024-09-06 20:52:22,211: INFO: pybricksdev.connections.pybricks: Connected successfully!
2024-09-06 20:52:22,240: DEBUG: bleak.backends.winrt.client: Read Characteristic 0005 : bytearray(b'3.5.0')
2024-09-06 20:52:22,270: DEBUG: bleak.backends.winrt.client: Read Characteristic 0007 : bytearray(b'1.4.0')
2024-09-06 20:52:22,299: DEBUG: bleak.backends.winrt.client: Read Characteristic 0009 : bytearray(b'\x01\x97\x03\x81\x00\x00\x00')
2024-09-06 20:52:22,329: DEBUG: bleak.backends.winrt.client: Read Characteristic 000F : bytearray(b'\x00\x02\x0f\x00\x00\x00\xc0\xfd\x03\x00')
2024-09-06 20:52:22,400: DEBUG: pybricksdev.compile: missing modules: []
2024-09-06 20:52:35,187: DEBUG: bleak.backends.winrt.client: max_pdu_size_changed_handler: 23
2024-09-06 20:52:35,188: DEBUG: bleak.backends.winrt.client: session_status_changed_event_handler: id: BluetoothLE#BluetoothLE60:e3:2b:4a:9e:7f-0c:74:e6:b4:8b:b0, error: <BluetoothError.SUCCESS: 0>, status: <GattSessionStatus.CLOSED: 0>
2024-09-06 20:52:35,189: INFO: pybricksdev.connections.pybricks: Disconnected!
2024-09-06 20:52:35,189: DEBUG: bleak.backends.winrt.client: closing requester
2024-09-06 20:52:35,195: DEBUG: bleak.backends.winrt.client: closing session
2024-09-06 20:52:35,197: INFO: pybricksdev.connections.pybricks: Disconnecting...
2024-09-06 20:52:35,197: DEBUG: pybricksdev.connections.pybricks: skipping disconnect because not connected
Traceback (most recent call last):
  File "<frozen runpy>", line 198, in _run_module_as_main
  File "<frozen runpy>", line 88, in _run_code
  File "C:\Python311\Scripts\pybricksdev.exe\__main__.py", line 7, in <module>
  File "C:\Python311\Lib\site-packages\pybricksdev\cli\__init__.py", line 389, in main
    asyncio.run(subparsers.choices[args.tool].tool.run(args))
  File "C:\Python311\Lib\asyncio\runners.py", line 190, in run
    return runner.run(main)
           ^^^^^^^^^^^^^^^^
  File "C:\Python311\Lib\asyncio\runners.py", line 118, in run
    return self._loop.run_until_complete(task)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "C:\Python311\Lib\asyncio\base_events.py", line 653, in run_until_complete
    return future.result()
           ^^^^^^^^^^^^^^^
  File "C:\Python311\Lib\site-packages\pybricksdev\cli\__init__.py", line 204, in run
    await hub.run(script_path, args.wait)
  File "C:\Python311\Lib\site-packages\pybricksdev\connections\pybricks.py", line 573, in run
    await self.download_user_program(mpy)
  File "C:\Python311\Lib\site-packages\pybricksdev\connections\pybricks.py", line 455, in download_user_program
    await self.client.write_gatt_char(
  File "C:\Python311\Lib\site-packages\bleak\__init__.py", line 786, in write_gatt_char
    await self._backend.write_gatt_char(characteristic, data, response)
  File "C:\Python311\Lib\site-packages\bleak\backends\winrt\client.py", line 906, in write_gatt_char
    await characteristic.obj.write_value_with_result_async(buf, response),
    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
OSError: [WinError -2147023673] The operation was canceled by the user

@laurensvalk
Copy link
Member Author

Ah, I forgot about that -- there is probably a remnant of an old program still on your hub 😄

This will normally automatically resets between new firmware versions, but the version hasn't been bumped yet.

I've been thinking to add a way for users to delete all their programs. Looks like a good use case for us developers/testers too.

@BertLindeman
Copy link
Contributor

So I should install a say 3.4 firmware first and re-try?

@laurensvalk
Copy link
Member Author

Try this version: firmware.zip.

@BertLindeman
Copy link
Contributor

Try this version: firmware.zip.

That helps! Thank you.

@BertLindeman
Copy link
Contributor

Some loose notes:

  • After install of your 3.6.0 firmware installed build 3540 without problem.

  • A warning about loosing a stored program might be in order later unless we preserve them over firmware changes, later.

  • the 5 stored programs is fun. Have seen an idea about some source indication which slot this program should go. Or do I invent this here?

  • The blinking heart status, I do not yet understand (probably forgot).
    Assume sending status info via appdata for the status display that is not yet in pybricks UI.
    Problem Naively, I tried to stop the blinking heart by long button press.
    The hub does not switch off and the press of left+right does no longer switch back to normal program mode.
    Battery-out is the only way I could get back.

So far for now.
Did not try any program larger than a few lines that display the slot I intended it to be.
Is there python access to "this program is loaded from slot-n"?

Thank you both for the fun with pybricks.

Bert

Copy link
Member

@dlech dlech left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would much prefer to see slots saved to external flash when not in use rather than trying to keep all slots in RAM at the same time. Otherwise, we are wasting a lot of RAM.

lib/pbio/sys/light_matrix.h Outdated Show resolved Hide resolved
lib/pbio/sys/light_matrix.c Outdated Show resolved Hide resolved
lib/pbio/sys/storage.c Outdated Show resolved Hide resolved
@laurensvalk
Copy link
Member Author

I would much prefer to see slots saved to external flash when not in use rather than trying to keep all slots in RAM at the same time. Otherwise, we are wasting a lot of RAM.

Sure, though in that case I suppose we should probably make the jump to a proper file system. In the mean time, you have to try quite hard to make 5x 20K sized programs, and you'll still have way more RAM remaining than what we have on any of the other hubs. Since this PR is a fairly small change, it seems like a step worth taking.

@laurensvalk
Copy link
Member Author

I think we could potentially set #define PBSYS_CONFIG_HMI_NUM_SLOTS (0) even on Prime Hub. This will let us merge this without making this part of the user experience just yet. Developers can then simply enable this flag to do experiments with Pybricks Code.

@BertLindeman
Copy link
Contributor

Just a remark after using the slots for some time.
I have a problem with remembering which slot I was working on.
Especially if the hub is not connected and times-out.
A reboot starts always on slot one, and I might be using another slot.
Oops..
My fault no doubt but sometimes still frustrating.

Maybe (very much) later an indication in the source to indicate which program slot this source is targetted?

@laurensvalk
Copy link
Member Author

Right - the thinking is to merge this with slots disabled until further notice 🙂

A reboot starts always on slot one, and I might be using another slot.

We could indeed make this persistent. For now the thinking was to always start at slot 0 so you can operate it without even seeing the display. (But I suppose that could still work if this was persistent.)

@laurensvalk
Copy link
Member Author

Merging with slot UI disabled on all hubs. Further discussion with various notes from here are collected in pybricks/support#1799

@BertLindeman
Copy link
Contributor

This is a closed issue, but I did not know what would be the best place to add this.
(In Dutch: "heb je hem weer", For David "Here he goes again")

Had this before 😉 but my impression was that it was fixed.

  • Installed 3.5.0 standard firmware in beta.pybricks.com on prime hub that was previously used to test program slots.
  • ran a test.
  • installed CI build 3565 on the same hub
  • tried to test, but the hub powers off.

installed the firmware from your remark here above

installed CI build 3565 over it and then all is OK.

Is the "old program data" still on the hub if we install a current CI build?
Assume only "enabled program slot testers" have this problem, so not too many.

@laurensvalk
Copy link
Member Author

Firmware released from now on will delete old programs. The 3.5.0 firmware did not.

So if download a program to a 3.6.0 firmware hub and then switch back to 3.5.0, the program still on the hub may not run, and shut down the hub. But you should be able to download new programs without issue.

So when you say:

tried to test, but the hub powers off.

Did you mean you just pressed the hub button to start the saved program, or did running new 3.5.0 programs fail?

@BertLindeman
Copy link
Contributor

Sorry for the confusion, Laurens.

Did you mean you just pressed the hub button to start the saved program, or did running new 3.5.0 programs fail?

Trying to run (F5) a new program on 3.6.0b1 did the power-off.
I did not try to start the "old" program.

For clarity and re-tested:

The program:

while True:
    pass
  • download 3.5.0 standard via beta.pybricks.com (over 3.6.0b1)
  • (F5) download a new program and run OK
  • download 3.6.0b1 over this 3.5.0
  • pull the USB cable (otherwise I do not see the hub power-off)
  • (F5) download a new program and run results (after ~ a second) in power-off.
    I see no animation on the display to indicate the program is running
  • then re-install 3.6.0b1 over 3.6.0b1 does not help.

While installing 3.6.0b1 I think that the phase "erasing old firmware" takes longer than on 3.5.0 and I assume that indicates there is more to be erased. So that is fine.

Bert

@laurensvalk
Copy link
Member Author

Thank you! I can reproduce something similar.

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.

4 participants