modm-io / modm

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

Incorrect module option availability generated for can buffers in documentation #804

Open joelsa opened 2 years ago

joelsa commented 2 years ago

At https://modm.io/reference/module/modm-platform-can-stm32-f0-f1-f2-f3-f4-f7-l4/, it is stated for the Rx and Tx buffer size option that

This option is only available for stm32{f0,f1,f3}.

Image showing the website documentation with the aforementioned error

But lbuild discover is giving the following output:

    │   ├── Module(modm:platform:can)   Controller Area Network (CAN)
    │   │   ├── Module(modm:platform:can:1)   Instance 1
    │   │   │   ├── Option(buffer.rx) = 32 in [1 .. 32 .. 65534]   
    │   │   │   ╰── Option(buffer.tx) = 32 in [1 .. 32 .. 65534]   

for my l4, f4, and f7 targets that have a can peripheral.

Also, @rleh showed me the following link where the option is added to all stm32 targets with a can peripheral: https://github.com/modm-io/modm/blob/develop/src/modm/platform/can/stm32/module.lb#L18-L30

So this seems to be an issue with the underlying magic:tm: that generates the documentation.

Unfortunately, the inner workings of the documentation generation are quite obscure to me, I'd guess the error origin is maybe somewhere around here: https://github.com/modm-io/modm/blob/develop/tools/scripts/generate_module_docs.py#L91-L123

If you think that this can be fixed by someone who knows python but is not at all acquainted with the docs generation I'd give fixing it a try, otherwise, I would be pretty grateful if someone else could take a look at this.

salkinium commented 2 years ago

Interesting bug. This is an issue in the generator script, which moves the options out of the instance nodes into the parent node so that the instance nodes do not spam the homepage. The solution I used basically picks the first mergeable option node it finds and discarding the rest, therefore the families are wrong. I'll take a look if I can reset the attached device filter to the parent node to remove this issue.

salkinium commented 2 years ago

Oh, wait it's actually correct, because the instances are completely ignored as are their options, and these families here have only one CAN peripheral without instances, therefore the options are in the :platform:can node and the filter is correct.

salkinium commented 2 years ago

The solution is to move the options upwards, but I didn't get to implement that, I only thought I already did.