Skip to content

MicroBitDisplay: move display refresh timer ownership to MicroBit#515

Merged
finneyj merged 1 commit intolancaster-university:masterfrom
MannuLambrichts:patch-timer4
Dec 17, 2025
Merged

MicroBitDisplay: move display refresh timer ownership to MicroBit#515
finneyj merged 1 commit intolancaster-university:masterfrom
MannuLambrichts:patch-timer4

Conversation

@MannuLambrichts
Copy link
Copy Markdown
Contributor

Summary

This pull request updates how the display refresh timer is instantiated and owned within the Codal display stack. Instead of allocating the timer inline inside the MicroBitDisplay constructor, the timer is now created in MicroBit and passed into MicroBitDisplay by reference, aligning it with how other Codal timers (system, adc, touch) are managed.

Motivation

Previously, MicroBitDisplay allocated the display refresh timer using new within its constructor. The resulting reference was not retained by MicroBitDisplay itself, relying instead on internal storage within NRF52LedMatrix. Since that member is private, the timer could not be explicitly managed or cleaned up. Also, NRF52LedMatrix does not allocate the timer, so it should not be responsible for maintaining it.
In rare (more advanced) scenarios, this could lead to orphaned timers and memory leaks.

Implementation details

  • The display refresh timer is now instantiated in MicroBit, similar to all other timers used by Codal.
  • MicroBitDisplay receives the timer via reference, making ownership and lifetime explicit.
  • The existing MicroBitDisplay constructor is preserved for compatibility but is marked as deprecated to discourage further use.
  • No functional behaviour of the display refresh logic is changed.

Create the display refresh timer in MicroBit and pass it to MicroBitDisplay, avoiding inline allocation and potential timer leaks.

Signed-off-by: Mannu Lambrichts <mannu346@gmail.com>
@finneyj
Copy link
Copy Markdown
Contributor

finneyj commented Dec 17, 2025

Thanks @MannuLambrichts - looks good!

@finneyj finneyj merged commit 6e70264 into lancaster-university:master Dec 17, 2025
13 of 16 checks passed
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.

2 participants