Skip to content

Move microrl token parsing arrays from the stack to the heap#1373

Merged
dexterbg merged 4 commits intoopenvehicles:masterfrom
leres:update
Apr 12, 2026
Merged

Move microrl token parsing arrays from the stack to the heap#1373
dexterbg merged 4 commits intoopenvehicles:masterfrom
leres:update

Conversation

@leres
Copy link
Copy Markdown
Contributor

@leres leres commented Apr 8, 2026

so we can increase _COMMAND_TOKEN_NMB (and allow more can capture filter terms without impacting stack usage).
Other fixed sized arrays in the module prevent more than about 128 from being helpful.

The changes to microrl_get_complite() are untested (and not used by ovms).

so we can increase _COMMAND_TOKEN_NMB (and allow more can capture
filter terms without impacting stack usage).
Other fixed sized arrays in the module prevent more than about 128
from being helpful.

The changes to microrl_get_complite() are untested (and not used
by ovms).
@dexterbg
Copy link
Copy Markdown
Member

dexterbg commented Apr 8, 2026

Craig,

  • malloc() allocates internal RAM, we should prioritize SPIRAM here. That's easily done using heap_caps_malloc_prefer().
  • You introduced a memory hole in microrl_get_complite().
  • Push the TAB key in the USB or SSH console to trigger microrl_get_complite(). TAB can be pushed anywhere in a begun registered command token, and will either complete the token if unique from the input so far, or list the matching tokens. If you want to learn about this, search where & how the get_completion callback is registered from our code base.

Regards,
Michael

Prefer SPIRAM over INTERNAL.
Fix memory leak in microrl_get_complite().
rl_get_complite() does not return a value.
@leres
Copy link
Copy Markdown
Contributor Author

leres commented Apr 8, 2026

If you prefer I can switch from compile time to run time SPIRAM detection; I believe the call looks something like this:

heap_caps_malloc_prefer(sizeof(*tkn_arr), _COMMAND_TOKEN_NMB, 
    MALLOC_CAP_DEFAULT | MALLOC_CAP_SPIRAM, 
    MALLOC_CAP_DEFAULT | MALLOC_CAP_INTERNAL);

@dexterbg
Copy link
Copy Markdown
Member

From esp_heap_caps.h:

#define MALLOC_CAP_SPIRAM           (1<<10) ///< Memory must be in SPI RAM
#define MALLOC_CAP_INTERNAL         (1<<11) ///< Memory must be internal; specifically it should not disappear when flash/spiram cache is switched off
#define MALLOC_CAP_DEFAULT          (1<<12) ///< Memory can be returned in a non-capability-specific memory allocation (e.g. malloc(), calloc()) call

So the correct call to prioritize SPIRAM is:

heap_caps_malloc_prefer(sizeof(*tkn_arr), _COMMAND_TOKEN_NMB, 
    MALLOC_CAP_SPIRAM,
    MALLOC_CAP_DEFAULT);

Regards,
Michael

@leres
Copy link
Copy Markdown
Contributor Author

leres commented Apr 11, 2026

I tried to bring my branch up to date but I suck too badly at git...

Here's the version that uses heap_caps_malloc_prefer()

@dexterbg
Copy link
Copy Markdown
Member

Looks good now, except for the inclusion of the canlog vfs component in the PR. I'd rather have this PR focusing on the microrl changes, and a second PR for the canlog changes -- and the canlog PR then should include all log formats. If this exceeds your git skills, it's also OK to include the canlog changes here, but they should then be complete (all formats).

As requested by @dexterbg. Also, the change is missing for other
cases (e.g. "can log start monitor crtd")
@dexterbg dexterbg merged commit 74c541e into openvehicles:master Apr 12, 2026
leres added a commit to leres/Open-Vehicle-Monitoring-System-3 that referenced this pull request Apr 14, 2026
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