lancaster-university / microbit-dal

http://lancaster-university.github.io/microbit-docs
Other
256 stars 130 forks source link

BLE Button A Characteristic + B-press causing OOM #434

Open SoylentGraham opened 5 years ago

SoylentGraham commented 5 years ago

I'm connecting my micro:bit to OSX, and interacting with CoreBluetooth. I can see it, and once paired, I can find all the services & characteristics, all good. I subscribe to E95DDA90-251D-470A-A062-FA1922DFA9A8 which is the characteristic UUID for button A. When I click, I get 0x1,0x2 (when held?) and back to 0x0. I can click it 100 times, all fine. If I click button B, I get the 020 :( display.

With a bit of testing, it seems like I have to click A a couple of times before B causes the crash. But clicking B is the trigger.

My sketch, sets the heart icon for A when pressed, and the ghost when I click B. There is nothing else in my sketch.

Firmware version (read from bluetooth) 2.1.1--g

Some more debugging may be needed (I'll remove the icons for example).

This is a new platform for me, so I haven't investigated the firmware etc, but I couldn't quite see any issues along these lines (at least not ones with firmware this recent)

martinwork commented 5 years ago

Can you share the code for the micro:bit hex? Adding Bluetooth to a hex uses a lot of RAM, each service initialised uses more and actually connecting via Bluetooth also uses more RAM. However I expect the OOM will have something to do with the handler calls for the button presses queuing up, with each queued handler call using some RAM. Could the button handler just set a variable which is used elsewhere to change the display?

SoylentGraham commented 5 years ago

I'm using the IOS app to build my "sketch", is there a way to get the hex file? (or share it in some way?)

I'll try and find some time to produce an even more minimal example. When I find that time I'll also try a different approach that doesn't crash :)

SoylentGraham commented 5 years ago

Aha! I can export the .hex, as a fellow developer, I have to commend the developers on how well & accessible you've made it. (especially generic things like the mass storage controller interface)

IMG_3886

Bluetooth buttons.hex.zip

martinwork commented 5 years ago

Your example crashed more easily than the one I tried because you have more services initialised. RAM is very limited when using Bluetooth. This is the sort of use of a variable I had in mind: Bluetooth-buttons-with-variable.zip. It's not doing quite the same thing but at least I can't make it panic!

A client app can use the event service to receive button press events. Then the hex wouldn't need the button service, saving RAM. A client can also send button press events.

SoylentGraham commented 5 years ago

So, does that mean the service is only allocated once it's invoked?

It felt like a bug to me, just because I assumed all the ram would be allocated when I initialise the services :) (and would expect a panic at bootup if too-many-services was the cause)

martinwork commented 5 years ago

The "on button A/B pressed" handlers are separate from the Bluetooth code. The button service creates its own MicroBitListener for each button's events. These things consume some RAM at boot, then need more as the program runs.

This C++ sample creates services just like the MakeCode blocks do: https://github.com/lancaster-university/microbit-samples/blob/master/source/examples/bluetooth-services/main.cpp#L94

The constructor for the button service is here: https://github.com/lancaster-university/microbit-dal/blob/master/source/bluetooth/MicroBitButtonService.cpp#L41

A listener may queue events: https://github.com/lancaster-university/microbit-dal/blob/master/source/drivers/MicroBitMessageBus.cpp#L96

I don't think the MicroBitButtonService listeners queue, do they? What about the MakeCode button handlers? https://github.com/Microsoft/pxt-microbit/blob/master/libs/core/input.cpp#L171

SoylentGraham commented 5 years ago

Thanks, I'm quite keen to dig into this now (when I have the time)

If you don't think this is an explicit bug, and just a general OOM case, feel free to close the issue :)