[feature] Nanopb scons integration

[lmoesch]

Here is the current draft of nanopb (protobuf) integration into modm. Currently there are some points that are still on my todo-list

• wait for new nanopb release tag that includes python path fix on windows
• rework description text for module and protofile path
• squash commits

[lmoesch]

Hey everyone - I know it's been quite a long time 😅. Nanopb pushed a new release "recently" and since I'm developing with modm again I'd like to look again into this issue.

@salkinium Did you had some insight with the integration in modm_tools?

[salkinium]

Did you had some insight with the integration in modm_tools?

No, not yet. I just finished my master thesis, so a lot of todos accumulated over the last six months. I would recommend to just leave it as it is right now, and later integrate it into the build systems. It's all just Python anyways.

[lmoesch]

Did you had some insight with the integration in modm_tools?

No, not yet. I just finished my master thesis, so a lot of todos accumulated over the last six months. I would recommend to just leave it as it is right now, and later integrate it into the build systems. It's all just Python anyways.

First things first: Congratulations on finishing your thesis!

I nevertheless dug further into the inner workings of scons and modm and I think I can manage to integrate it atleast as part of the scons build. This would have mostly the benefit of having a ways of simply rebuild the protocol without rebuilding modm and it could automatically be updated on scons build.

Let me hear your thoughts on the following setup:

• The nanopb modm-repo is responsible for providing the basic headers (maybe via modm-tools) and signals that nanopb is used.
• A nanopb compiler script is added to scons/site-tools which adds a new Builder for nanopb (e.g. simply scons nanopb) which could also be part of the general build (not that easy because of chain dependencys so some Emittermagic is required).

[salkinium]

Let me hear your thoughts on the following setup:

Yeah, that's pretty much how I would do it.

There is some emitter magic in the XPCC generator, which also generates C++ code from an abstract description via a Python script.
But I remember there's also a SCons builder already part of the nanopb project itself, perhaps copying that would be better?

[lmoesch]

Let me hear your thoughts on the following setup:

Yeah, that's pretty much how I would do it.

There is some emitter magic in the XPCC generator, which also generates C++ code from an abstract description via a Python script. But I remember there's also a SCons builder already part of the nanopb project itself, perhaps copying that would be better?

Jeah I had a quick look into the builder in nanopb/tests which looks pretty sophisticated and I will try to use it for code generation.

[salkinium]

Just hack it until it works somewhat, I honestly don't fully understand SCons either even though I wrote most of our setup…

[lmoesch]

Sometimes I'd like to question my life choices when it's already 4am and you find out, that the problem was not your code, but a bug in nanopb for windows which they had fixed years ago, but still not put into their latest release version....

Nevertheless I made quite progress with the integration. I could not use their scons integration directly, but I cherry picked most of it's functions into an own Builder. I will update this PR when I have a working example.

[salkinium]

This looks really good! Do you want to add a small example to have the CI test it?

[lmoesch]

Finally I see no erros in the CI 😅 But I tryed another more sophisticated example and it fails due to wrong build order of files. I have to dig into it before squashing everything together

[salkinium]

With a working example, I can add support to the other build systems and test it against SCons.

[lmoesch]

Okay ... I found a solution ... but ...

[lmoesch]

@salkinium any idea why the stm32f4-examples 2 pieline fails? I know I put the nanopb example in there, but it looks like this would cause another example to crash?

[lmoesch]

@salkinium any idea why the stm32f4-examples 2 pieline fails? I know I put the nanopb example in there, but it looks like this would cause another example to crash?

Okay, nevermind. Nanopbs SCons integration is even more broken on Linux than on Windows ... I'm not exactly sure how their tests can be "passing" when they complete fail when checked out on a local machine ...

[lmoesch]

@salkinium any idea why the stm32f4-examples 2 pieline fails? I know I put the nanopb example in there, but it looks like this would cause another example to crash?

Alright. I've been running the CI code in a local docker a couple of time now and AFAICT there is some kind of race-condition, because everytime I do a clean checkout I get different errors, e.g:

scons: building terminated because of errors.arm-none-eabi-g++: error: modm/src/modm/math/utils/bit_operation.cpp: No such file or directory

or

scons: building terminated because of errors.arm-none-eabi-g++: error: modm/src/modm/platform/clock/systick_timer.cpp: No such file or directory

By running

../tools/scripts/examples_compile.py nucleo_f429zi/nanopb

When running the command often enough it builds after 2-3 times. And it's always other files that are "missing".

When running Scons directly in the directoy it fails compiling the ext/gcc repo due to missing files, which "fixes" itself by running the command three times.

No idea what is going on there. The dependency injection from nanopb just adds the generated *.cpp files to the sources and flags them dependant on the nanopb header (Which is the dirty fix because normally this would be part of the compile step). Funny thing is on my Windows hostmachine it builds without any issue 🙈

[lmoesch]

Jeah I feel kinda genius for figuring out what the f**** is happening... nanopbs action builder uses chdir for running the command which is - NOT THREADSAFE. So all other commands that run parallel are also moved to this directory. Thatswhy it cannot find the files anymore, because they are now relative to the protocol directory ....

I thought it was a good Idea to import code from their scons directly but apparently I end up rewriting every function now ...

[lmoesch]

🍾🍾🍾

Finally ... I do some cleanup tomorrow and will then squash everything together, what a mess 😂😂😂

[salkinium]

I've added some changes to the docs and the SCons wrapper, mostly removing the dependency injection, which I think is done better this way.
I also added a output path to the generator to put it in line with the other generators.

Is there only one protofile to include, or do you expect the PathOption to take multiple paths? Cos that would require a module.add_set_option(PathOption) to enable multiple comma-separated paths in the projects.xml. If you want to support that, the example should be augmented.

[lmoesch]

I've added some changes to the docs and the SCons wrapper, mostly removing the dependency injection, which I think is done better this way. I also added a output path to the generator to put it in line with the other generators.

Is there only one protofile to include, or do you expect the PathOption to take multiple paths? Cos that would require a module.add_set_option(PathOption) to enable multiple comma-separated paths in the projects.xml. If you want to support that, the example should be augmented.

Heys! I would prefer the option to add multiple paths, but I did not know how to do it :'D I will add this then ;)

[salkinium]

I added the set option and added a new protofile to the example, which needs to be adapted further.
There's still an issue with the cmdstr of the CommandAction, which I'm trying to fix.

[salkinium]

There's still an issue with the cmdstr of the CommandAction, which I'm trying to fix.

Fixed now, was very simple with env.subst.

[salkinium]

I've extended the example, but I couldn't get the separate .options file to work, so the options need to be specified inline (which I find better anyways).
As far as I could tell the protoc compiler just ignores any .options file and I couldn't find any tests in nanopb that uses the options non-inline. So I guess their docs are out of date or something like that.

I would consider this PR now complete, can you take a look if something is missing?

[lmoesch]

Hey @salkinium Great job with reworking my nanopb-ripoff code :) In my eyes this PR is better then ever and has everything in it that I still had in mind (especially the linked protofiles).