pybricks-micropython pybricks.tools: Add hostbuffer to receive IDE data

This adds a class that allows a user program to allocate a buffer for receiving data from an IDE.

It has similarities to receiving data over standard in, but it is a dedicated separate command. The data is not queued but (over)written to a fixed size buffer. This is a bit like downloading a program, but with user-readable data.

This is useful for applications where the buffer contains "sensor data"-like contents prepared by the IDE.

Usage is as follows:

from pybricks.tools import HostBuffer

# Allocates buffer 4 bytes. From here, the program is ready to receive
# data write commands to this buffer from a connected host like Pybricks Code. 
buffer = HostBuffer("hbb")

# Returns tuple with 3 values (int16, int8, int8)
values = buffer.get_values()

# Returns bytes copy of the 4-byte buffer.
data = buffer.get_bytes()

# gc collection of the buffer will automatically stop processing
# received data write commands. If Pybricks Code sends more, they
# are just ignored.

Before merging this as-is, it might be useful to thinking about the complementary case as well, where the user program can write data back to the IDE. This could facilitate things like live sensor port view without messing with stdout.

May 09 '24 08:05 laurensvalk

coverage: 56.205% (-0.4%) from 56.563% when pulling 13c77cca8e921e7bd9ef458bc6705e3ab502e04c on hostbuffer into 79a0e010ac36fb64593d5ef5b213b64f1a03985b on master.

May 09 '24 08:05 coveralls

Download the artifacts for this pull request:

May 09 '24 08:05 github-actions[bot]

HostBuffer sounds like a very technical name. Have you been programming too much? :laughing:

Maybe we could call it something a little less technical like CommsData?

(Being more technical in the Pybricks BLE protocol side of things is good though)

May 09 '24 14:05 dlech

Thanks for the name suggestions. I was thinking this could be the base class for various applications, e.g. BallSensor, giving x, y, width, height as % of the canvas, with bbbb format. These could probably live in the user code or as a separate module instead of in the firmware.

The challenge with CommsData is that it's maybe less clear where the data is coming from. If we do that do, maybe AppData works too since it is coming from the Pybricks Code app.

May 09 '24 14:05 laurensvalk

maybe AppData works too since it is coming from the Pybricks Code app.

I like it.

May 09 '24 14:05 dlech

Before merging this as-is, it might be useful to thinking about the complementary case as well, where the user program can write data back to the IDE. This could facilitate things like live sensor port view without messing with stdout.

Looking ahead, since AppData doesn't imply a direction, maybe the other way around could be part of the same class.

Or possibly something like AppDataReceiver for this one and AppDataSender for the potential future counterpart since it will work rather differently.

Higher level uses cases that could make use of a lower level AppDataSender:

sensor "port view" (this is requested a lot)
variable monitor (sometimes requested)

May 09 '24 14:05 laurensvalk

Looking ahead, since AppData doesn't imply a direction, maybe the other way around could be part of the same class.

Makes sense.

# rx-only
app = AppData("b")
# same as
app = AppData(rx="b")

# tx-only
app = AppData(tx="b")

# bidirectional
app = AppData("b", "b")
# same as
app = AppData(rx="b", tx="b")

# get latest
x, = app.rx()

# send
app.tx(1)

May 09 '24 16:05 dlech

Depending on the buffer sizes we think could be used, it might be nice to somehow specify an offset, index, or slice, or something like it when reading and writing so you only have to send relevant or new data.

For example

app = AppData("bbbHhh")
distance = app.rx(index=3) # reads the value corresponding to H

May 09 '24 17:05 laurensvalk

Not as pretty, but can already do that if it returns a tuple:

distance = app.rx()[3]

May 09 '24 17:05 dlech

That's how it currently implemented, and probably fine for reading if we assume buffers will stay relatively small. For sending, the buffer may potentially be larger than the MTU, so some kind of index or offset could be needed.

It could be the data points of a SLAM model or something. Unless that could just be a separate use case, with sequential output like data logging instead of a fixed size buffer.

May 09 '24 18:05 laurensvalk

maybe the other way around could be part of the same class

AppData.send(bytes_data)

could be the sending side of this. Since the PC has infinite resources, we don't have to pre-allocate any buffers or use offsets. It would pretty much work like raw standard output, but it wouldn't be printed to the output pane.

Jul 11 '24 08:07 laurensvalk

Are there any builds/versions where the AppData is also implemented for the bidirectional transfer?

As far as I understand current implementation of HostBuffer is able to receive data, but is not able to transmit.

Jul 16 '24 20:07 afarago

That is not implemented yet. It would probably work a bit like pbsys_bluetooth_tx*, but with a new PBIO_PYBRICKS_EVENT_WRITE_APPDATA = 2 to augment the existing PBIO_PYBRICKS_EVENT_WRITE_STDOUT = 1 so Pybricks Code can tell them apart and not display the incoming app data.

In the firmware, it could perhaps have a separate buffer, similar to stdout_ring_buf. Or maybe there is a way to do it with a single ring buffer.

* Separately, in the bigger picture, pbsys_bluetooth_tx probably needs to become something like pbsys_stdio_tx so we can generalize it to BLE + USB.

Jul 18 '24 08:07 laurensvalk

Or maybe there is a way to do it with a single ring buffer.

Since this is only transmitting/receiving a fixed sized state, no ring buffer should be needed.

If we want to guarantee that every state change is sent, the write should be a blocking/awaitable function.

The ring buffer for stdout is mainly to improve throughput of print() since it has variable-sized data (and also makes it non-blocking for small enough data rates)

Jul 18 '24 14:07 dlech

the write should be a blocking/awaitable function.

Excellent point, thanks. Yes, this should keep it fairly simple.

Jul 18 '24 15:07 laurensvalk

the write should be a blocking/awaitable function.

Excellent point, thanks. Yes, this should keep it fairly simple.

Since the Move Hub (MTU 23) probably won't support these features anyway, we could keep this even simpler by restricting outgoing messages to PBDRV_CONFIG_BLUETOOTH_MAX_MTU_SIZE - 1 bytes in size. The next minimum is City/Technic at 158 - 1 bytes.

@afarago , what is the largest message you would typically need for a single portview information message? Is 157 bytes acceptable?

Jul 22 '24 12:07 laurensvalk

Just pushed an in-progress version that includes sending. The following works:

from pybricks.tools import AppData

app = AppData("bb")
print(app.get_values())

print("Seen on stdout...")
app.write_bytes(b"Won't see this, but it will be sent!")  # This is also awaitable

stdout messages are labeled with PBIO_PYBRICKS_EVENT_WRITE_STDOUT = 1, while the app data output is labeled with PBIO_PYBRICKS_EVENT_WRITE_APP_DATA = 2 so it doesn't pollute the terminal pane.

Each line is also guaranteed to be sent in one chunk rather than occasionally buffered like stdout, so this should be easier to parse on the Pybricks Code side.

I'd like to clean up the firmware side implementation a bit though. Not entirely happy with how it works with btstack at the moment.

Jul 22 '24 19:07 laurensvalk

coverage: 56.025% (-0.6%) from 56.592% when pulling ef9f7fddae77c544ea876e2e77a5cf1ab9aaec87 on hostbuffer into d92da744c581b4d87d30e492c85ab637f7f7e50a on master.

Jul 22 '24 19:07 coveralls

Since the Move Hub (MTU 23) probably won't support these features anyway, we could keep this even simpler by restricting outgoing messages to PBDRV_CONFIG_BLUETOOTH_MAX_MTU_SIZE - 1 bytes in size. The next minimum is City/Technic at 158 - 1 bytes.

I think so. At the moment based on your suggestion I am going with one message per device, that is "BBll" for port number, port type, value1, value2, ... we might go on with this. For the hub, we still have character data, but 157 shall be more than enough.

Jul 23 '24 15:07 afarago

Sending longer AppData messages on Technic Hub fails. I think it boils down to the following:

bStatus_t ATT_HandleValueNoti(uint16_t connHandle, attHandleValueNoti_t *pNoti) {
    uint8_t buf[32]; // <------- This seems very small. Should it be 158 - 3 + 5 = 160?

    buf[0] = connHandle & 0xFF;
    buf[1] = (connHandle >> 8) & 0xFF;
    buf[2] = 0; // authenticated link not required
    buf[3] = pNoti->handle & 0xFF;
    buf[4] = (pNoti->handle >> 8) & 0xFF;
    memcpy(&buf[5], pNoti->pValue, pNoti->len);

    return HCI_sendHCICommand(ATT_CMD_HANDLE_VALUE_NOTI, buf, 5 + pNoti->len);
}

What do you think, @dlech ?

I'm also wondering if this could potentially also explain some other related printing issues.

EDIT: If I change the 32 byte buffer size above it won't crash anymore, but still not send anything at all if you try to send more than 20 bytes. So something else is still capping the messages at 20 bytes, even though this is not using the stdout ringbuffer of exactly that size.

Jul 26 '24 14:07 laurensvalk

Assuming that the central has a MTU >= the hub is probably a fairly safe assumption, but it is entirely possible that the central could be something like a Move Hub with max MTU of 23 bytes.

So ideally, the AppData Python object would be aware of the negotiated MTU and use that to calculate the max size rather than using the hub's hard-coded max value.

As for the CC2640 chips, it could very well be that the 20 byte max payload for notifications is hard-coded in the Bluetooth chip firmware. But I do see a GATT_UpdateMTU() function, so maybe we need to call this after the MTU exchange to tell the Bluetooth chip resize it's notification buffer?

I'm also wondering if this could potentially also explain some other related printing issues.

The stdout buffer is chunked into 20 bytes (MAX_CHAR_SIZE) so wouldn't be overflowing the 32 byte buffer or exceeding the limit you found when trying to send more, if that is what you mean.

Jul 26 '24 22:07 dlech

Assuming that the central has a MTU >= the hub is probably a fairly safe assumption, but it is entirely possible that the central could be something like a Move Hub with max MTU of 23 bytes.

So ideally, the AppData Python object would be aware of the negotiated MTU and use that to calculate the max size rather than using the hub's hard-coded max value.

As for the CC2640 chips, it could very well be that the 20 byte max payload for notifications is hard-coded in the Bluetooth chip firmware. But I do see a GATT_UpdateMTU() function, so maybe we need to call this after the MTU exchange to tell the Bluetooth chip resize it's notification buffer?

:+1:

I think I'll adjust this PR to use MAX_CHAR_SIZE for each chunk, and then we can look at improving throughput separately.

I'm also wondering if this could potentially also explain some other related printing issues.

The stdout buffer is chunked into 20 bytes (MAX_CHAR_SIZE) so wouldn't be overflowing the 32 byte buffer or exceeding the limit you found when trying to send more, if that is what you mean.

I was thinking of https://github.com/pybricks/support/issues/1152, but maybe this is just a different issue. That should indeed be protected by the MAX_CHAR_SIZE.

Jul 27 '24 09:07 laurensvalk

Hello, I have a few questions about AppData. 1.is the sending of the messages from the hub only available on beta.pybricks.com? 2. How can I send a list between the hub and pc? 3. How can the pc recieve the list in a regular python script?

Aug 27 '24 19:08 techcatgato