`piconim build` : Append appropriate parameter to `target_link_libraries()` in CMakeLists

[casey-SK]

Whenever a picostdlib module such as adc or pwm is imported, it should be noticed and appended to the target_link_libraries() in CMakeLists during the build operation.

[Martinix75]

And yes, you always need to add them in the examples that I have always put the note to add to Cmake "Add hardware_xx to cmake"

[beef331]

Ideally when you import the module it automates it, this probably can be done with CTE writing to the file, or possibly by emitting the values to the stdout which can be picked up by piconim. A method that does it once is the best to reduce string operations, though the compile time evaluation method will probably work fine.

[casey-SK]

I am also imagining a complex program that will import a picostdlib module outside of the programs main module, and having to make sure we catch that with piconim. This is a problem I am interested in tackling this week, unless you wanted to do it haha.

[beef331]

I think we should just have a CTE we call in the modules that need a special link like linkLibrary("hardware_adc") which will modify the file. Or write to a file in the root of the project which is used then deleted to store the needed flags. I'm open to any idea to solve this.

[auxym]

This is sort of related to #32, because this implies that piconim build would run cmake, which is not the case currently (cmake gets run once on init), which was the first solution I though of for correctly handling .gitignore.

The issue I see with running cmake on build is that cmake needs to know the SDK path, or whether to download the SDK. So either a --sdk command is passed to piconim build every time, or it is passed once (maybe with the setup command I propose in #32?) and store in some config file.

[casey-SK]

So far, when I use one of the "extra" modules (such as pwm), I just append hardware_pwm to the target_link_libraries() line in the CMakeLists.txt file. I do this after piconim init has been called and I just run piconim build

Which means I alter the CMakeLists.txt file and run make , and it works fine...

I am far from an expert in C haha, I just know this runs

[casey-SK]

Update: I have been working to solve this problem and noticed that the current implementation of piconim build does not support more than on module. I have fixed that problem locally and will include it in this issues PR

[auxym]

So far, when I use one of the "extra" modules (such as pwm), I just append hardware_pwm to the target_link_libraries() line in the CMakeLists.txt file. I do this after piconim init has been called and I just run piconim build

Which means I alter the CMakeLists.txt file and run make , and it works fine...

I am far from an expert in C haha, I just know this runs

Yes, you could just ask devs to modify the CMakeLists file manually. I thought the idea of this issue was to have piconim build do it automatically. I don't mind one way or the other, personally.

[auxym]

Forgot to mention: devs would also have to re-run cmake after modifying CMakeLists.

1. cmake generates the Makefile based on CMakeLists.
2. make runs the build based on Makefile
3. piconum runs make.

[casey-SK]

here is how I fixed the problem: https://github.com/casey-SK/picostdlib/commit/28d9e754a874251ecc1e563dfdf3f0a1aa23c02b Let me know if it is satisfactory and I will create a PR.

[beef331]

Ideally we have a seamless import and build system overtop cmake/make, we can store the sdk path in a file in the base of the directory which can be git ignored, but either way manually adding things is dumb we're in 2022 not 1922 :smiley: I really want a solution for this that is clean and effective, so I'm thinking hard.

[casey-SK]

1. https://github.com/beef331/picostdlib/pull/35#issuecomment-1007813390, I am only looking at files that originated as a @m ... .nim.c , which seems to be the ones that were built when running nim c -c .... And I can't tell of another way of determining the imports, as in a nim file, you can put an import statement almost anywhere.
2. Thanks to @auxym for working on our cmake woes.

[beef331]

My present idea is that in modules that require new link libraries we add requiresLink("hardware_adc") for example, which writes to a file somewhere that piconim cares about(current working directory, since we'll delete it anyway), we then iterate over that file like you did before compiling, adding all the entries to the cmake list. This makes it so we dont have to deal with searching Nim files or C files.

[auxym]

import-with-side-effects, as described by @beef331 was my first idea too.

The other is parsing the generated C files with a minimalistic parser to find the #include <header>.h, and add the links depending on which headers are referenced by the nim code. Could very well be massive overengineering (I'm not sure how minimal the parser could be and still reliably find the includes).

[beef331]

Well if we want to iterate files we only have to iterate the directory we place the C files and search for path.endsWith("@spicostdlib@ssrc@spicostdlib@sstdio.nim.c") for instance as far as I know.

[casey-SK]

My present idea is that in modules that require new link libraries we add requiresLink("hardware_adc") for example, which writes to a file somewhere that piconim cares about(current working directory, since we'll delete it anyway), we then iterate over that file like you did before compiling, adding all the entries to the cmake list. This makes it so we dont have to deal with searching Nim files or C files.

The only thing I don't like about requiresLink() is that it is another simple thing to forget and then your program won't run and may not give you a useful error message. If we wanted to give them a useful error message we would need to parse the files for the import statements, much like the other option we are discussing.

[beef331]

The user doesnt add them, we add them in the modules. So importers need not worry.

[casey-SK]

oh, I see what you mean

[Martinix75]

Hi guys, I tried the new system to include libraries (hardware_pwm), while before it was easy (it was enough to add it in cmakelists.txt) now frankly I can't, too difficult for an electronic (maybe for a programmer it's simple), or at least I can not do it. What maybe some operations to do? If you could describe them? Or are there any automatism I ignore? I'm sorry but this system for me is too complex! (I preferred the other .. for the moment). Sorry for the pot!

[beef331]

It hasnt changed yet afaik, it should still just be a text file, if this goes through it should be automatic and you wont need to add it.

[Martinix75]

Maybe it's just my fault that I can't do! I don't even fill out "hello_pio" !! For now I go back to 0.2.7 then he was going! For today they are already sad! thanks anyway! I'll shoot over it! Thank you for now!

[Martinix75]

This I tried all day, but manually inserting "hardware_pwm" in cmakelists.txt, I always get the same mistake! I do not know what to do! Go beyond my electronic acquisences!

In file included from /home/andrea/PicoNim/cazzo/csource/build/nimcache/cazzo.c:5: /home/andrea/PicoNim/cazzo/csource/nimbase.h:276:35: error: static assertion failed: "backend & Nim disagree on size for: PwmChannel{enum} [type declared in /home/andrea/.nimble/pkgs/picostdlib-0.2.11/picostdlib/pwm.nim(23, 3)]" 276 | #define NIM_STATIC_ASSERT(x, msg) _Static_assert((x), msg) | ^~~~~~ /home/andrea/PicoNim/cazzo/csource/build/nimcache/cazzo.c:25:1: note: in expansion of macro 'NIM_STATIC_ASSERT' 25 | NIM_STATIC_ASSERT(sizeof(enum pwm_chan) == 4, "backend & Nim disagree on size for: PwmChannel{enum} [type declared in /home/andrea/.nimble/pkgs/picostdlib-0.2.11/picostdlib/pwm.nim(23, 3)]");

error: static assertion failed: "backend & Nim disagree on size for: PwmChannel{enum} --> worrying !!!!!!!!!!!!!!!!!!

I don't know why but I think it's a problem in the library, because "hardware_i2c" for example it works !!! (tested now)

[auxym]

I'm pretty sure this is related to the fact that I added the --checkAbi flag to the compiler command line in #46. It's a good thing though, as it indicates that the nim object def is not compatible with the C def, which would probably have caused a crash, or silently wrong behavior, at run time. That's why I though it was a good idea to add it.

As for fixing the issue, I can have a look tonight of tomorrow, we probably just need to add {.size:.} with the correct size to the enum def.

[beef331]

Yea it's a good thing to have(didnt know about it before), and I think the PWM issue has been fixed with the recent version I pushed, just think @Martinix75 has the older version.

[Martinix75]

Great! Now with Pisdtdlib 0.2.12 "PWM" compile!!. But I found another problem (excuse me !!) Similar to the previous one, but on IRQ!

/home/andrea/ProgPicoNim/pwm2/csource/build/nimcache/@m..@s..@s..@s.nimble@spkgs@spicostdlib-0.2.12@spicostdlib@sgpio.nim.c:32:1: note: in expansion of macro 'NIM_STATIC_ASSERT' 32 | NIM_STATIC_ASSERT(sizeof(enum gpio_irq_level) == 4, "backend & Nim disagree on size for: IrqLevel{enum} [type declared in /home/andrea/.nimble/pkgs/picostdlib-0.2.12/picostdlib/gpio.nim(136, 3)]"); | ^~~~~ make[2]: *** [CMakeFiles/pwm2.dir/build.make:76: CMakeFiles/pwm2.dir/nimcache/@m..@s..@s..@s.nimble@spkgs@spicostdlib-0.2.12@spicostdlib@sgpio.nim.c.obj] Error 1

NIM_STATIC_ASSERT(sizeof(enum gpio_irq_level) == 4, "backend & Nim disagree on size for: IrqLevel{enum} [type declared in /home/andrea/.nimble/pkgs/picostdlib-0.2.12/picostdlib/gpio.nim(136, 3)]");

I don't know if it's a common and solvable thing with some tricks, or if it just concerns a few things! I can only limit myself to reporting. (the program that from this error with 0.2.12, compile in properly with 0.2.7) ! Good day!

[beef331]

Same issue, I clumsily thought that the size of enums in C were the size of a cint so I need to fix it everywhere.