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

simple program out of memory error (possible music issue?) #3868

Closed
DaveAtKitronik opened this issue Feb 4, 2021 · 5 comments · Fixed by #4481
Closed

simple program out of memory error (possible music issue?) #3868

DaveAtKitronik opened this issue Feb 4, 2021 · 5 comments · Fixed by #4481

Comments

@DaveAtKitronik
Copy link
Contributor

A customer reported an error using the Kitronik smart greenhouse. His system gives compilation error.
If he removed the sensing blocks (the board has a BME280 on it) then the error went but the code was not functional.
we could not reproduce so asked him to send the actual code.
The Java representation is:

input.onButtonPressed(Button.A, function () {
music.startMelody(music.builtInMelody(Melodies.PowerUp), MelodyOptions.Once)
kitronik_smart_greenhouse.controlHighPowerPin(kitronik_smart_greenhouse.HighPowerPins.pin13, kitronik_smart_greenhouse.onOff(true))
basic.pause(100)
kitronik_smart_greenhouse.controlHighPowerPin(kitronik_smart_greenhouse.HighPowerPins.pin13, kitronik_smart_greenhouse.onOff(false))
})
let zipLEDs = kitronik_smart_greenhouse.createGreenhouseZIPDisplay(3)
kitronik_smart_greenhouse.setBuzzerPin()
kitronik_smart_greenhouse.setTime(0, 0, 0)
zipLEDs = kitronik_smart_greenhouse.createGreenhouseZIPDisplay(8)
let zipStick = zipLEDs.zipStickRange()
basic.forever(function () {
zipLEDs.setZipLedColor(0, kitronik_smart_greenhouse.colors(ZipLedColors.Red))
zipLEDs.setZipLedColor(1, kitronik_smart_greenhouse.colors(ZipLedColors.Green))
zipLEDs.setZipLedColor(2, kitronik_smart_greenhouse.colors(ZipLedColors.Blue))
zipLEDs.show()
basic.showString(kitronik_smart_greenhouse.readTime())
if (kitronik_smart_greenhouse.readIOPin(kitronik_smart_greenhouse.PinType.analog, kitronik_smart_greenhouse.IOPins.p0) > 500) {
basic.showIcon(IconNames.Happy)
} else {
basic.showIcon(IconNames.Sad)
}
basic.pause(1000)
basic.clearScreen()
basic.showString("P:")
basic.showNumber(kitronik_smart_greenhouse.pressure(PressureUnitList.mBar))
basic.pause(500)
basic.clearScreen()
basic.showString("T:")
basic.showNumber(kitronik_smart_greenhouse.temperature(TemperatureUnitList.C))
basic.pause(500)
basic.clearScreen()
basic.showString("H:")
basic.showNumber(kitronik_smart_greenhouse.humidity())
basic.pause(500)
basic.clearScreen()
zipStick.showColor(kitronik_smart_greenhouse.colors(ZipLedColors.Red))
basic.pause(2000)
zipStick.showColor(kitronik_smart_greenhouse.colors(ZipLedColors.Green))
basic.pause(2000)
zipStick.showColor(kitronik_smart_greenhouse.colors(ZipLedColors.Blue))
basic.pause(2000)
})

You will also need the Kitronik smart greenhouse extension loaded.
Not a very complex program. We have written more complex in the testing of the board before release.
We instructed him how to look at output.txt - this reports 'Everything seems fine!'

trying to compile his code I managed to catch a less optimistic output.txt which reported:
error: pxt_modules/pxt-kitronik-smart-greenhouse/main.ts(1,1): error TS9200: program too big by 1968 bytes!

which I have not seen before in a PXT program.

digging a bit further I loaded our test program:

input.onButtonPressed(Button.A, function () {
basic.showString(kitronik_smart_greenhouse.readTime())
basic.pause(500)
basic.showString("T:")
basic.showNumber(kitronik_smart_greenhouse.temperature(TemperatureUnitList.C))
basic.showString("C")
basic.pause(500)
basic.showString("H:")
basic.showNumber(kitronik_smart_greenhouse.humidity())
basic.showString("%")
basic.clearScreen()
})
input.onButtonPressed(Button.AB, function () {
if (!(lampOn)) {
lampOn = true
zipStick.showColor(kitronik_smart_greenhouse.colors(ZipLedColors.Yellow))
} else {
lampOn = false
zipStick.clear()
zipStick.show()
}
})
input.onButtonPressed(Button.B, function () {
kitronik_smart_greenhouse.controlHighPowerPin(kitronik_smart_greenhouse.HighPowerPins.pin13, kitronik_smart_greenhouse.onOff(true))
basic.pause(200)
kitronik_smart_greenhouse.controlHighPowerPin(kitronik_smart_greenhouse.HighPowerPins.pin13, kitronik_smart_greenhouse.onOff(false))
})
let soilHue = 0
let humidHue = 0
let tempHue = 0
let lampOn = false
let zipStick: kitronik_smart_greenhouse.greenhouseZIPLEDs = null
kitronik_smart_greenhouse.setDate(20, 1, 20)
kitronik_smart_greenhouse.setTime(15, 0, 0)
let zipLEDs = kitronik_smart_greenhouse.createGreenhouseZIPDisplay(8)
let statusLEDs = zipLEDs.statusLedsRange()
zipStick = zipLEDs.zipStickRange()
zipLEDs.setBrightness(255)
lampOn = false
basic.forever(function () {
tempHue = Math.map(kitronik_smart_greenhouse.temperature(TemperatureUnitList.C), 0, 40, 210, 0)
humidHue = Math.map(kitronik_smart_greenhouse.humidity(), 0, 100, 35, 150)
soilHue = Math.map(kitronik_smart_greenhouse.readIOPin(kitronik_smart_greenhouse.PinType.analog, kitronik_smart_greenhouse.IOPins.p1), 0, 1023, 35, 150)
statusLEDs.setZipLedColor(0, kitronik_smart_greenhouse.hueToRGB(tempHue))
statusLEDs.setZipLedColor(1, kitronik_smart_greenhouse.hueToRGB(humidHue))
statusLEDs.setZipLedColor(2, kitronik_smart_greenhouse.hueToRGB(soilHue))
statusLEDs.show()
})

(again with the Kitronik smart greenhouse extension)

This compiles and runs fine.
examination showed that the customer uses a music block. removing this allows his code to compile correctly.
looking in the build folder there is binary.js, which appears to have sizes of programs in the top of it.
The original customer code reported:
// total=182551 new=8.16% cached=89.30% other=2.54%
removing just the music block (startMelody) drops the size significantly:
// total=117016 new=11.99% cached=85.01% other=3.00%

suspiciously the difference is 65535. (bytes?)

Adding in the playMelody block, with an 8 note tune increases it back to:
// total=174870 new=8.44% cached=88.91% other=2.66%
still smaller, (57845 different) - small enough that it will compile with essentially the same functionality.

Our greenhouse test code reports:
// total=122867 new=13.41% cached=83.48% other=3.12%
and adding the startMelody block to that gives:
// total=188402 new=9.21% cached=88.16% other=2.63%
which also produces the error.

I have advised the customer of this workaround, but these are not (on the face of it) complex programs.
If our extension is to large then suggestions on how to minimise its size would be appreciated.

I have not found a definitive max size anywhere - if this info is available it would be useful.

micro:bit version (please complete the following information):
not hardware related - doesnt get as far as the download.

Desktop (please complete the following information):

  • OS: windows
  • Browser chrome and safari - not thought to be browser or OS related.
@abchatra
Copy link
Collaborator

Can you please provide a shorter repro and reopen the issue?

@microbit-carlos
Copy link
Collaborator

A simple way to reproduce:

This code:

basic.showIcon(IconNames.Heart)

binary.js:

// total=16139 new=8.85% cached=78.06% other=13.09%

And adding a startMelody block:

basic.showIcon(IconNames.Heart)
music.startMelody(music.builtInMelody(Melodies.Dadadadum), MelodyOptions.Once)

binary.js:

// total=81455 new=2.79% cached=93.24% other=3.97%

This adds 65316 bytes (81455 - 16139), just shy of 64KBs.

Is this expected? Can this be reduced?

@jaustin jaustin reopened this Aug 16, 2021
@microbit-carlos
Copy link
Collaborator

For comparison, the playMelody block only consumes an extra 59067 bytes.

basic.showIcon(IconNames.Heart)
music.playMelody("- - - - - - - - ", 120)
// total=75206 new=2.87% cached=92.81% other=4.32%

@riknoll
Copy link
Member

riknoll commented Mar 1, 2022

I expect that getMelody() is the culprit. I'll see if I can rewrite it using hexbuffers

@riknoll
Copy link
Member

riknoll commented Mar 2, 2022

Well, I was able to improve it a bit by switching from arrays to hex buffers (68531 -> 57546). Unfortunately, I'm not sure I can get much better than that without changing the API itself. We could probably improve both code size and memory usage if we switched to single strings instead of arrays of strings; but that would be a breaking change in TS (we could make it transparent in blocks)

Something to consider when evaluating #4454

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment