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.

Sign up for GitHub

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

Jump to bottom

midi sad face 20 error #1281

Closed
machinehistories opened this issue Sep 22, 2018 · 34 comments
Closed

midi sad face 20 error #1281

machinehistories opened this issue Sep 22, 2018 · 34 comments
Assignees
@abchatra
Labels
bug next-release

Comments

@machinehistories
Copy link

I am running midi bluetooth on the windows app and in the browser. This is code that worked in the past. Now it almost immediately gives the 20 error code after midi connection is made. I am running 0243 firmware.
microbit-midit (7).zip
@martinwork
Copy link
Contributor

Here is another example. Just Bluetooth, not MIDI.

The attached zip contains three versions of the same project:

original - created in January
live - imported and re-saved today at https://makecode.microbit.org/
beta - saved from https://makecode.microbit.org/beta

original - works as expected
live - panic 020 as soon as flashed
beta - panic 020 when a gamepad handler is triggered

@pelikhan
Copy link
Member

Where is the zip?

@pelikhan
Copy link
Member

@finneyj FYI

@martinwork
Copy link
Contributor

Good point! A bit of a memory problem?
outofmemory.zip

@pelikhan
Copy link
Member

Looks like pxt-bluetooth-midi has compilation issues against v1. investigating.

@pelikhan
Copy link
Member

pelikhan commented Sep 24, 2018 •
edited
Loading

We have

midi/midi.cpp
/home/build/prj2/source/bluetooth-midi/midi.cpp: In function 'void bluetooth::midiSendMessage(pxt::Buffer)':
/home/build/prj2/source/bluetooth-midi/midi.cpp:7:41: error: 'class pxt::BoxedBuffer' has no member named 'payload'
 #define PXT_BUFFER_DATA(buffer) buffer->payload

which means the macro at https://github.com/Microsoft/pxt-microbit/blob/20500fb9aea2d46c475402ab685b0e79bacb264f/libs/core/platform.h#L6 was not used. @mmoskal is platform.h properly included in microbit?

@pelikhan
Copy link
Member

Other extensions impacted: https://github.com/search?q=PXT_BUFFER_DATA&type=Code

@pelikhan pelikhan added the bug label Sep 24, 2018
@pelikhan
Copy link
Member

Looking into this

@jaustin
Copy link
Collaborator

jaustin commented Sep 24, 2018

@pelikhan I think this isn't just V1 related, as Martin's example suggests that something's up in V0 too.

@machinehistories can you please confirm that you're using the 'live' version of makecode at makecode.microbit.org rather than the beta?

Thanks
Jonny

This was referenced Sep 24, 2018
Closed
Merged
@machinehistories
Copy link
Author

machinehistories commented Sep 24, 2018 via email

@pelikhan
Copy link
Member

On v1, it does not even build so you get a broken program from MakeCode. I have a fix coming.

@pelikhan
Copy link
Member

There's two issues that are being mixed up here.

  • A compilation issue on v1 for the midi package (fix coming)
  • OOM issues with BLUE in v0/v1 using the 2.1DAL

FOr the memory issue, what is Lancaster saying about memory pressure in DAL 2.1?

@pelikhan
Copy link
Member

Compile issue fixed and deployed 088c956

@pelikhan pelikhan assigned abchatra and unassigned mmoskal Sep 24, 2018
@finneyj
Copy link

finneyj commented Sep 24, 2018

Thanks @pelikhan

Since microbit-dal 2.1.0, we've moved to using a single heap memory allocator. Previously, dynamic memory allocation was split between the MicroBitHeapAllocator and the gcc/libc heap allocator. The reasons why are legacy. In 2.1.0, we move purely to the MicroBitHeapAllocator.

There are two observations we noted from this.

  1. there were far more background allocations happening than we were ever aware of (as we couldn't easily track behaviour of system code allocation previosuly).

  2. the libc/gcc allocator was over allocating. It would happily allocate a user memory in STACK space when it ran out of heap memory... which is very unsafe, as at some later point the code would very likely crash unpredictably.

The new allocator won't do that - it will protect whatever memory is reserved for the stack, and OOM with an 020 panic if memory is exhausted... So i suspect that's what's hapenning in these cases. Code that previously was unsafe and worked sometimes, is now being trapped as thus and prevented...

@pelikhan If you remember, we tweaked the stack size for BLE builds in MakeCode recently to help ease these problems a little. Do you know if those changes will be applying to the code in this issue?

@finneyj
Copy link

finneyj commented Sep 24, 2018

hmm... looks like it is:

        "optionalConfig": {
            "microbit-dal": {
                "stack_size": 1280,
                "gatt_table_size": "0x700"
            }
        },

0x700 is a large GATT table size for a simple example though. That's probably using up about 1K more than is needed... I wonder if there's a way to reduce this in a way that's not too restricting.

@pelikhan
Copy link
Member

Right so in the past we maxed out the GATT table in order to support "all BLE services on" scenario.

@finneyj
Copy link

finneyj commented Sep 24, 2018

yep. The dal will reclaim any unused space though...

@pelikhan
Copy link
Member

So we were lucky in the past...

@machinehistories
Copy link
Author

machinehistories commented Sep 25, 2018 via email

@finneyj
Copy link

finneyj commented Sep 25, 2018 •
edited
Loading

@pelikhan yes, I think so...

@machinehistories I hear you. I'm just trying to explain the reasons here as a start point. We should look to resolve these issues of course...

@pelikhan - my spidey sense tells me this is close to running. As a stopgap, is there a way to set the GATT table size differently for different packages? i.e. have the midi package set a small GATT table size in config.json than the "full" bluetooth package?

@jaustin
Copy link
Collaborator

jaustin commented Sep 25, 2018 •
edited
Loading

@pelikhan would I expect changing the 'project settings' to include the following to help me debug?

"yotta": {
    "config": {
        "microbit-dal": {
            "stack_size": 500,
            "gatt_table_size": "0x400"
        }
    }
}

(based on https://github.com/Microsoft/pxt-microbit/blob/master/libs/bluetooth/pxt.json#L23)

I tried this on a little memory consumption program to see if I could see any difference and whether I use a gatt_table_size of 0x900 or 0x400 I don't see any difference in the results (BLE versus non BLE is huge though: ~1000 iterations to ~180!)

microbit-gobbler.zip

And @finneyj if this does work I think we could enable heap debugging easily on a MakeCode project by the same method.

Here's my attempt to characterise this https://makecode.microbit.org/_f8J5T6HtxLTV

@pelikhan
Copy link
Member

A package can override the gatt_table_size but there is no mechanism to handle clashing of multiple request. e.g. what if two packages request a different gatt size? do we take the max?

@pelikhan
Copy link
Member

If you have a YOTTA config flag for heap debugging, you could enable it from the pxt.json config section.

@jaustin
Copy link
Collaborator

jaustin commented Sep 25, 2018

For now I'm just trying to debug this - are you saying things set in the project settings should work then? Do you mind checking the URL above to see that I've done what you'd expect because they both seem to behave the same way (but then maybe my test fills stack not heap?)

@pelikhan
Copy link
Member

If you build locally with docker, you call look at the dockeryt folder. It contains the generated module.json... which is the ultimate truth about which flags ended being enabled in the config.

@pelikhan
Copy link
Member

The MIDI build issues covered. General memory consumption issues from BLE should be handled in the microbit-dal side.

@finneyj
Copy link

finneyj commented Sep 26, 2018

agree - let's track over there and see what we can do

lancaster-university/microbit-dal#390

@jaustin
Copy link
Collaborator

jaustin commented Sep 26, 2018

@machinehistories please don't consider the closure of the issue here on MakeCode as a suggestion we're not going to keep looking into this - it's just we need to move the issue to a different project to track it - as it's the DAL that can resolve the memory consumption issue (we may need MakeCode to update some parameters or settings once we've sorted out the right conditions, but we should debug at a DAL level).

@jaustin
Copy link
Collaborator

jaustin commented Sep 27, 2018

@machinehistories can you try this program and see if it works for you? It doesn't run out of memory as fast for me here (if I mash the buttons fast I can still trigger an 020)
https://makecode.microbit.org/_UHCE2EU8hhgs

I can't actually test the midi side though! I'm testing using the nRF Go app, so it's a bit uncertain that it's a real case
If that doesn't work you can try this one, which is an absolutely tiny stack, but doesn't modify the gatt table
https://makecode.microbit.org/_F0uhgEiu88tc
(it'll crash but not with an 020, probably)
These all have memory debugging enabled, so if you connect a terminal you will be able to see the allocations as they happen.

@machinehistories
Copy link
Author

machinehistories commented Sep 27, 2018 via email

@jaustin
Copy link
Collaborator

jaustin commented Sep 27, 2018

Hey, both of those links are good to compile against the live build, yes. I've modified the project settings to change the stack and heap size. It means they don't build as fast as normal (normally they're not rebulding the whole thing) so if it spins for a while before download give it time (up to a minute or two!)

@machinehistories
Copy link
Author

machinehistories commented Sep 28, 2018 via email

@machinehistories
Copy link
Author

The way I am testing these is using midiberry on windows 10
on android I am using midi ble connect
on ios midimittr
Again I appreciate all you hard work developing a better app but I also have a lesson plan and using midi bluetooth is a part of that. I just want to be able to show the students the cool things they can do so please do what you can so the students can do cool stuff. I am happy to try anything you want to send me. I am going to mention one other thing. I doubt it is related but on the off chance I might as well mention it.
When pairing on windows 10 I would first delete all microbits from the list of devices in order to perform a fresh repairing sequence. After connecting to the currently powered microbit, even though the other boards weren't powered or connected and they were recently deleted from my list of bluetooth devices, they would also show back up as paired. This seemed unusual. Not sure if there is a cache that needs to be flushed or if this is an issue but I don't remember that happening in the past.

@jaustin jaustin mentioned this issue Oct 2, 2018
Open
@lock
Copy link

lock bot commented Sep 28, 2019

This thread has been automatically locked since there has not been
any recent activity after it was closed. Please open a new issue for
related bugs.

@lock lock bot locked as resolved and limited conversation to collaborators Sep 28, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees

@abchatra abchatra

Labels
bug next-release
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
7 participants
@jaustin @martinwork @pelikhan @abchatra @mmoskal @finneyj @machinehistories