sys/auto_init: improve documentation by aabadie · Pull Request #9328 · RIOT-OS/RIOT

aabadie · 2018-06-12T07:08:44Z

Contribution description

This PR is a follow-up of #9264 and fixes #7001. It's not adding a new dedicated markdown page but just improving the actual auto_init module doxygen documentation.

Issues/PRs references

fixes #7001, related to #9264

miri64 · 2018-06-12T07:30:40Z

sys/include/auto_init.h

- *              when using auto_init unless you know what you're doing.
+ *
+ * This feature can be enabled in any application by adding the `auto_init`
+ * module to the application `Makefile`:


"to the application's Makefile"

miri64 · 2018-06-12T07:35:29Z

sys/include/auto_init.h

+ * ~~~~~~~~~~~~~~~~~~~~~~~
+ *
+ * `auto_init` initializes any included module that provides auto
+ * initialization capabilities.


Below, you use "auto-initialization" instead of "auto initialization". I prefer the first, but regardless: please be consistent.

miri64 · 2018-06-12T07:36:27Z

sys/include/auto_init.h

+ * `auto_init` initializes any included module that provides auto
+ * initialization capabilities.
+ *
+ * Drivers or cpu peripherals that provides a SAUL adaption and network


Maybe add a @ref here to each of the groups below?

aabadie · 2018-06-12T08:32:07Z

@miri64, first batch of comments addressed.

I also have other doxygen changes in all drivers that provide saul adaption (but not commited/pushed): in each driver header, I added @ingroup drivers_saul and a line mentioning the drivers provides saul capabilities. This way, all drivers with saul support are listed in the SAUL documentation page. Do you think it's worth and it can be added to this PR ?

miri64 · 2018-06-12T09:14:52Z

Do you think it's worth and it can be added to this PR ?

That sounds rather like new PR material ;-).

miri64 · 2018-06-12T09:32:11Z

sys/include/auto_init.h

+ *
+ * The default initialization parameters can be overriden by the application in
+ * several ways (examples with the @ref drivers_bmp180 oversampling parameter
+ * `BMP180_PARAM_OVERSAMPLING`):


I think all three paragraphs above should be merged, since they are independent from the very first paragraph, but belong thematically very close together. Also, I think the first one of these three is hard to read in rendered docs, due to using auto-refs. Please consider the following

Device drivers that provides a [SAUL](@ref drivers_saul) adaptation and [network device drivers](@ref drivers_netdev) can be initialized automatically using by the `auto_init` module. Their default initialization parameters need to be in a `DRIVER_params.h` file provided by the driver implementation. They can be overriden by the application in several ways (examples below with the @ref drivers_bmp180's oversampling parameter `BMP180_PARAM_OVERSAMPLING`):

miri64 · 2018-06-12T09:34:06Z

sys/include/auto_init.h

+ *  - by passing them via the `CFLAGS` variable on the build command line`:
+ *
+ * ```
+ * CFLAGS=-DBMP180_PARAM_OVERSAMPLING=1 USEMODULE=bmp180 make BOARD=arduino-zero -C examples/default


For some reason default is bold in the rendered doc. Don't know why though... using ~~~~~~~~~~~~~~~~ {.sh} instead doesn't help :-/.

miri64 · 2018-06-12T09:34:25Z

sys/include/auto_init.h

+ * - by just setting the `CFLAGS` variable in the application `Makefile`:
+ *
+ * ~~~~~~~~~~~~~~~~~~~~~~~ {.mk}
+ * CFLAGS +=-DBMP180_PARAM_OVERSAMPLING=1


Space messing between = and -

miri64 · 2018-06-12T09:35:05Z

sys/include/auto_init.h

+ * several ways (examples with the @ref drivers_bmp180 oversampling parameter
+ * `BMP180_PARAM_OVERSAMPLING`):
+ *
+ *  - by passing them via the `CFLAGS` variable on the build command line`:


Stray ` at end of line

miri64 · 2018-06-12T09:35:33Z

sys/include/auto_init.h

+ * CFLAGS=-DBMP180_PARAM_OVERSAMPLING=1 USEMODULE=bmp180 make BOARD=arduino-zero -C examples/default
+ * ```
+ *
+ * - by just setting the `CFLAGS` variable in the application `Makefile`:


Remove "just" here. It doesn't fit in the middle of the enumeration.

miri64 · 2018-06-12T09:36:38Z

sys/include/auto_init.h

+ * ~~~~~~~~~~~~~~~~~~~~~~~
+ *
+ * - by copying the `bmp180_params.h` header to the application directory and
+ * editing it there with the desired values (not recommended). This file will


Since when is this not recommended? It's the only way to do it if you add additional external devices (say e.g. a samr21-xpro with an Openlabs connected).

miri64 · 2018-06-12T09:38:19Z

sys/include/auto_init.h

- *              USEMODULE in the application's Makefile. auto_init will initialize
- *              any other included module that does not require a parameter in
- *              its init function, i.e. if the prototype looks like this: void
- *              MODULE_init(void). Most timer modules or simple drivers can be


This whole section about the format of the prototype disappears, but is as important as the parameterization of complex drivers added below. It should go before that.

miri64 · 2018-06-12T09:39:11Z

sys/include/auto_init.h

+ *
+ * - by copying the `bmp180_params.h` header to the application directory and
+ * editing it there with the desired values (not recommended). This file will
+ * be included first and, thanks to header guards, the one from the driver


The half-sentence about header guards sounds clunky. Remove?

aabadie · 2018-06-12T19:30:02Z

@miri64, I reworked the documentation.

aabadie · 2018-06-13T07:32:40Z

That sounds rather like new PR material ;-).

see #9337

aabadie · 2018-06-18T11:35:27Z

@miri64, I think your comments are addressed here. May I squash ?

miri64

Yes, please squash.

aabadie · 2018-06-18T12:08:09Z

@miri64, squashed.

aabadie · 2018-06-19T07:58:23Z

All green there, so I think I can merge.

aabadie added Type: enhancement The issue suggests enhanceable parts / The PR enhances parts of the codebase / documentation Area: doc Area: Documentation CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR labels Jun 12, 2018

aabadie requested a review from miri64 June 12, 2018 07:08

miri64 requested changes Jun 12, 2018

View reviewed changes

aabadie force-pushed the pr/sys/auto_init_doc_enh branch from afd346c to 2da81e8 Compare June 12, 2018 19:28

aabadie mentioned this pull request Jun 13, 2018

drivers*: add drivers with saul support in the doxygen drivers_saul group #9337

Merged

miri64 approved these changes Jun 18, 2018

View reviewed changes

sys/auto_init: improve documentation

741c8c0

aabadie force-pushed the pr/sys/auto_init_doc_enh branch from 2da81e8 to 741c8c0 Compare June 18, 2018 11:59

aabadie merged commit d49e0d2 into RIOT-OS:master Jun 19, 2018

cladmi added this to the Release 2018.07 milestone Jul 10, 2018

aabadie deleted the pr/sys/auto_init_doc_enh branch September 23, 2018 09:00

Conversation

aabadie commented Jun 12, 2018

Contribution description

Issues/PRs references

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

aabadie commented Jun 12, 2018

Uh oh!

miri64 commented Jun 12, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

aabadie commented Jun 12, 2018

Uh oh!

aabadie commented Jun 13, 2018

Uh oh!

aabadie commented Jun 18, 2018

Uh oh!

miri64 left a comment

Choose a reason for hiding this comment

Uh oh!

aabadie commented Jun 18, 2018

Uh oh!

aabadie commented Jun 19, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants