modm-io / modm

modm: a C++23 library generator for AVR and ARM Cortex-M devices
https://modm.io
Mozilla Public License 2.0
720 stars 128 forks source link

[ext] Adding ext json library and example #1146

Closed hshose closed 2 months ago

hshose commented 4 months ago

Added the nlohmann json library, actually the single header include version.

I more or less copied what other ext modules do with their update script and all. Feel free to move this into the modm-ext github namespace.

Example creates and retrieves data from plain JSON and BSON written to FLASH, see #1145.

Probably heaps of room for improvement in the example, I wasn't really sure if I should use the modm::accessor::Flash since it doesn't do anything on the G4 as far as I could see?

salkinium commented 4 months ago

Very nice!

I've created https://github.com/modm-ext/json-partial and made you a maintainer. Please push your repository there, add a smol README and make sure the GH actions keeps the repo up-to-date (see this update.yml for an example). Please move the submodule to ext/nlohmann/json and put the lbuild file in ext/nlohmann/module.lb. This aligns it with the other submodules. (We even have a secret guide for adding external code like this :-)

chris-durand commented 4 months ago

Nice!

I wasn't really sure if I should use the modm::accessor::Flash

modm::accessor::Flash is only relevant for AVR where flash and RAM don't share the same address space.

hshose commented 4 months ago

Another thing, that @rleh brought up yesterday and @chris-durand originally mentioned in his reply to #1145 (if I understood correctly, please correct me if not): Would it be reasonable to integrate a "persistent" flash section at the end of flash in the linkerscript (size configured with lbuild option)?

For my STM32G474 example, resulting in something like:

MEMORY
{
    FLASH (rx) : ORIGIN = 0x08000000, LENGTH = 522241
    PERSISTENT_FLASH (rx) : ORIGIN = 0x807f801, LENGTH = 2047
        /*... etc ... */
}

__flash_start = ORIGIN(FLASH);
__flash_end = ORIGIN(FLASH) + LENGTH(FLASH);
__persistent_flash_start = ORIGIN(PERSISTENT_FLASH);
__persistent_flash_end = ORIGIN(PERSISTENT_FLASH) + LENGTH(PERSISTENT_FLASH);
/*... etc ... */

I would probably do this based on an lbuild option; Something like a :platform:cortex-m:persistent_flash_size that defaults to zero.

Would this be done inside modm-devices or the cortex-m linkerscript?

E.g., here in the linkerscript add something like

%% for memory in memories
%% if memory.name == "FLASH" and options[":platform:cortex-m:persistent_flash_size"] is 0
{{ memory.name | upper }} ({{ memory.access }}) : ORIGIN = {{ "0x%08X" % memory.start }}, LENGTH = {{ memory.size - options[":platform:cortex-m:persistent_flash_size"] }}
PERSISTENT_FLASH (rx) : ORIGIN = {{ "0x%08X" % (memory.start + memory.size - options[":platform:cortex-m:persistent_flash_size"])}}, LENGTH = {{options[":platform:cortex-m:persistent_flash_size"]}}
%% else
{{ memory.name | upper }} ({{ memory.access }}) : ORIGIN = {{ "0x%08X" % memory.start }}, LENGTH = {{ memory.size }}
%% endif
%% endfor

and similarly here to get the __persistent_flash_start and __persistent_flash_end.

This is my first time touchign a modm linkerscript and its a bit intimidating.

Also not sure, if this is even a reasonable feature for modm to have?

salkinium commented 4 months ago

Also not sure, if this is even a reasonable feature for modm to have?

Yes, we already have an offset of the FLASH start address in the linkerscript to accommodate bootloaders: https://github.com/modm-io/modm/blob/c43be03e82b22f5980e50415a855bee10ee70c59/src/modm/platform/core/cortex/module.lb#L232-L239 Your application is basically the same, but from the other side. (You cannot use the front, since that's where the vector table needs to be placed). Remember to validate the option so that it doesn't collide with the flash offset option: https://github.com/modm-io/modm/blob/c43be03e82b22f5980e50415a855bee10ee70c59/src/modm/platform/core/cortex/module.lb#L279-L294

I'd call it :platform:cortex-m:linkerscript.flash_reserved analog to :platform:cortex-m:linkerscript.flash_offset.

hshose commented 4 months ago

Thanks @salkinium for the advice! This was really helpful.

I have implemented the flash_reserved section and some validation (that flash_reserved is not bigger than flash_size - flash_offset).

Seems to work in the example.

salkinium commented 4 months ago

Thanks, I will review this weekend in depth.

hshose commented 4 months ago

Thank you @salkinium for all the fixes and help with this! I'll test more on hardware tomorrow; I have G474 and G431 devboards here and if I remember correctly there, should also be someⓇ G0 devboard somewhere™.

salkinium commented 4 months ago

FYI: No pressure, but if you want this in the 2024q1 release, it needs to be merged by monday evening.

salkinium commented 4 months ago

(Squashed and rebased.)

hshose commented 3 months ago

Yeah sorry this didn't happen until last night unfortunately.

I have tested on G474 and G431 now and it works on both boards. I coulnd't find a G0 board to test with :cry:

I also took the liberty to rename the example from json to flash_json, because half the example is actually about using flash not about using the JSON library.

salkinium commented 3 months ago

I lost track, is this ready to merge @rleh?

salkinium commented 2 months ago

macOS CI is broken again, I'm so tired of that crap.

hshose commented 2 months ago

Thanks for merging!!! :heart: :cupid: