Skip to content

Conversation

@dhalbert
Copy link
Collaborator

@dhalbert dhalbert commented Sep 3, 2025

Detailed changes:

  • For cardputer_keyboard.c, allocate DemuxKeyMatrix it creates and the objects it holds (all the way down) on the port heap, because it lives past VM's.
  • To do the above, add support for non-VM heap for DemuxKeyMatrix and the keypad allocations it needs by passing an arg use_gc_allocator, as is done in lvfontio.
  • Add mp_obj_port_malloc(), mp_obj_port_malloc_var(), mp_obj_new_port_tuple(), all of which allocate on the port heap, not the VM heap.
  • Add port_malloc_zero(), because port_malloc() did not zero out its allocation, unlike VM heap allocations. I got caught by this.
  • Move port_gc_collect() from main.c to supervisor/port.c for consistency. This function is not actually used by anyone, but I was considering using it.
    -Add an MP_WEAK mp_board_init(), for board-specific initializations needed when a VM starts. Also considered, but not used. Added anyway for possible future use.
  • Add an MP_WEAK board_gc_collect(), for board-specific gc. Not used.
  • Don't duplicate keyboard code between boards/m5stack_cardputer and boards/m5stack_cardputer_ros. Instead, add a ports/espressif/module directory for this code, in the style of a port-specific module directory, like the port-specific bindings directories.

My original approach to this was to make the cardputer_keyboard object be allocated when the VM started up. That required less pervasive changes, and it worked fine, except for the Press any key... when the VM finished. So I had to make it live across VM's.

I tested and @dglaude also tested (thanks!).

@dhalbert dhalbert force-pushed the cardputer-keyboard-fix branch from 49024ef to c121d73 Compare September 3, 2025 01:41
- For `cardputer_keyboard.c`, allocate `DemuxKeyMatrix` it creates and the objects it holds (all the way down) on the port heap, because it lives past VM's.
- To do the above, add support for non-VM heap for `DemuxKeyMatrix` and the `keypad` allocations it needs by passing an arg `use_gc_allocator`, as is done in `lvfontio`.
- Add `mp_obj_port_malloc()`, `mp_obj_port_malloc_var()`, `mp_obj_new_port_tuple()`, all of which allocate on the port heap, not the VM heap.
- Add `port_malloc_zero()`, because `port_malloc()` did not zero out its allocation, unlike VM heap allocations. I got caught by this.
- Move `port_gc_collect()` from `main.c` to `supervisor/port.c` for consistency. This function is not actually used by anyone, but I was considering using it.
- Add an `MP_WEAK` `mp_board_init()`, for board-specific initializations needed when a VM starts. Also considered, but not used. Added anyway for possible future use.
- Add an `MP_WEAK` `board_gc_collect()`, for board-specific gc. Not used.
- Add an `MP_WEAK` `port_gc_collect()`, for port-specific gc. Not used.
- Don't duplicate keyboard code between `boards/m5stack_cardputer` and `boards/m5stack_cardputer_ros`. Instead, add a `ports/espressif/module` directory for this code, in the style of a port-specific `module` directory, like the port-specific `bindings` directories.
@dhalbert dhalbert force-pushed the cardputer-keyboard-fix branch from c121d73 to de2efe0 Compare September 3, 2025 02:08
Avoids the new Wunterminated-string-literal when compiled with gcc 15.1.

It would be preferable to just disable this warning, but Clang
-Wunknown-warning-option kicks in even when disabling warnings so this
becomes fiddly to apply.

This work was funded through GitHub Sponsors.

Signed-off-by: Angus Gratton <angus@redyak.com.au>
Avoids the new Wunterminated-string-literal when compiled with gcc 15.1.

This work was funded through GitHub Sponsors.

Signed-off-by: Angus Gratton <angus@redyak.com.au>
@dhalbert
Copy link
Collaborator Author

dhalbert commented Sep 3, 2025

I cherry-picked two commits from micropython#17269 to fix a gcc 15 build problem on Windows.

@dhalbert dhalbert requested a review from tannewt September 3, 2025 14:08
Copy link
Member

@tannewt tannewt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One minor thing. Thanks!

Comment on lines 48 to 54
// Do-nothing so not all boards need to provide this function.
MP_WEAK void mp_board_init(void) {
}

// Do-nothing so not all boards need to provide this function.
MP_WEAK void board_gc_collect(void) {
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you actually need these? I'd love to move away from MP_WEAK since these can be used even when not expected. In other words, the compiler doesn't check for correctness here. A #define to enable the function would be compile checked.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't need them, so I'll just remove them. I just spent the time figuring out where to put them, but I can come back to the commit to find that.

I left port_gc_collect() because it was already there.

I'll open an issue about replacing MP_WEAK functions with some guards instead. Did you mean that one might accidentally replace a weak function by mistake?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did you mean that one might accidentally replace a weak function by mistake?

Nope, you may think you are overriding one but don't actually. So, it'll compile and link but not run the implementation you expected. (Lots of IDF issues for us were this.)

@dhalbert dhalbert requested a review from tannewt September 3, 2025 23:54
Copy link
Member

@tannewt tannewt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you!

@tannewt tannewt merged commit 17ea151 into adafruit:main Sep 4, 2025
1249 of 1250 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.

M5Stack Cardputer bootloop with CircuitPython 10.0.0-beta.2

3 participants