Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

doc/porting-boards.md: improve with porting graph and reference section #15981

Merged
merged 2 commits into from Sep 16, 2021

Conversation

miri64
Copy link
Member

@miri64 miri64 commented Feb 10, 2021

Contribution description

The main objective of this PR is to provide a porting flowchart similar to the one AWS FreeRTOS provides (but way simpler, since we have a HAL). For the color scheme I oriented myself on other graphs (like the netdev description or the one on the gnrc main page).

Since we lack some guides however ("How to port a CPU?", "How to make a network device compatible with a network stack?") I added some TODO notes for those to the be provided so I have something to link to.

Regarding the "How to port a CPU?" part @akshaim has already something in the pipeline AFAIK, @jia200x, @fjmolinas should we look into the "How to make a network device compatible with a network stack?" part?

Testing procedure

Read the doc generated by CircleCI. When clicking the rectangle action nodes in the graph, you will be lead to the corresponding guide.

Issues/PRs references

None.

@miri64 miri64 added Type: enhancement The issue suggests enhanceable parts / The PR enhances parts of the codebase / documentation Area: doc Area: Documentation labels Feb 10, 2021
@miri64 miri64 added the CI: skip compile test If set, CI server will run only non-compile jobs, but no compile jobs or their dependent jobs label Feb 10, 2021
@MrKevinWeiss
Copy link
Contributor

Can we either drop the todos and add them in a tracking issue or just implement them?

Whenever I read todo in others documentation I always get scared off.

@MrKevinWeiss
Copy link
Contributor

Other than that I think it is helpful.

@waehlisch
Copy link
Member

when i click on the rectangles, i get an error message. maybe this is because of CircleID?

rest looks good to me!

@miri64 miri64 changed the title Doc/enh/porting graph doc/porting-boards.md: improve with porting graph and reference section Feb 11, 2021
@miri64
Copy link
Member Author

miri64 commented Feb 11, 2021

when i click on the rectangles, i get an error message. maybe this is because of CircleID?

Yeah, seems like a problem of CircleCI specifically in this case. Since I link it statically, CircleCI seems to try to access its static store for that link, so that might be the problem here.

But I can have a look, if one can reference a page dynamically from a dot file. As the name of the output HTML might change (and it also won't work for a LaTeX-generated PDF) this might be desirable anyway.

@miri64
Copy link
Member Author

miri64 commented Feb 11, 2021

Whenever I read todo in others documentation I always get scared off.

Will remove the @todo from the netdev page, but keep the one for CPUs for now, since I expect a PR from @akshaim for that soon. I am a bit worried though, that not having a porting guide in netdev but referring to it in the graph might also confuse people. But confusion may be less harmful being scared off :-).

@miri64
Copy link
Member Author

miri64 commented Feb 11, 2021

But I can have a look, if one can reference a page dynamically from a dot file.

Using @ref <page-name> seems to work just fine even in an imported dot file.

@miri64
Copy link
Member Author

miri64 commented Feb 11, 2021

But I can have a look, if one can reference a page dynamically from a dot file.

Using @ref <page-name> seems to work just fine even in an imported dot file.

Done that and also did some style fixes to the dot file. Now, if porting-cpus.md would be removed it would generate an error and the link would just lead to the generated SVG of the graph itself.

@miri64
Copy link
Member Author

miri64 commented Feb 11, 2021

Can you have another look @waehlisch, please, once CircleCI is done building the artifact?

@akshaim
Copy link
Member

akshaim commented Feb 11, 2021

Hey @miri64 ,

Instead of the 'END", can you have it as "Blink the LED's" and link it to LED tests . Afterall its not really the end 😉

@miri64
Copy link
Member Author

miri64 commented Feb 11, 2021

Instead of the 'END", can you have it as "Blink the LED's" and link it to LED tests

I thought the long-term plan was to get rid of the LEDx_* macros (see #15931). Since this is the final test, wouldn't linking to the hello-world example be a better way (and also be more "traditional", because let's be honest, every guide should at least contain a "Hello World" ;-))

@miri64
Copy link
Member Author

miri64 commented Feb 11, 2021

Afterall its not really the end 😉

That makes it even more funny, because then the graph would literally end in a "Hello" ;-).

@akshaim
Copy link
Member

akshaim commented Feb 11, 2021

Instead of the 'END", can you have it as "Blink the LED's" and link it to LED tests

I thought the long-term plan was to get rid of the LEDx_* macros (see #15931). Since this is the final test, wouldn't linking to the hello-world example be a better way (and also be more "traditional", because let's be honest, every guide should at least contain a "Hello World" ;-))

Ending on a "Hello World" sounds cool but making a board print hello world over serial might take more time than blinking the LED's which is pretty straight forward , doesn't event need timers :)

@miri64
Copy link
Member Author

miri64 commented Feb 11, 2021

Ending on a "Hello World" sounds cool but making a board print hello world over serial might take more time than blinking the LED's which is pretty straight forward , doesn't event need timers :)

Mhhh, the porting guide mentions the LED macros within board.h but only in half a sentence, so the dev might not even see the LEDs blinking either. hello-world does not need timers either (when the stdio does not need it). But maybe, when the LED macros are gone, the test will be ported to either dbgpin or just straight periph_gpio, so OK.

@miri64
Copy link
Member Author

miri64 commented Feb 11, 2021

Can you have another look @waehlisch, please, once CircleCI is done building the artifact?

Links still don't work I just noticed :-/. But they do locally when build with make doc.

@miri64
Copy link
Member Author

miri64 commented Feb 11, 2021

Instead of the 'END", can you have it as "Blink the LED's" and link it to LED tests . Afterall its not really the end wink

Done

Comment on lines +343 to +344
- [In her blog][martines-blog], Martine Lenders documented her approach of
porting the @ref boards_feather-nrf52840 in February 2020.
Copy link
Member Author

@miri64 miri64 Feb 11, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

BTW feel free to provide more examples that I could integrate here. Feels a bit pretentious to only cite my own blog here ;-).

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it is great!

Copy link
Member

@akshaim akshaim Feb 15, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I did a porting guide for EFM32HG "CPU". Could be useful for similar ones as well. The porting guide does not include documentation on the generator tools used by @basilfx though.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why not adding this documentation as a concrete example for porting to the doxygen?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@akshaim I can link your guide. How would you prefer it to be cited here?

Copy link
Member

@akshaim akshaim Feb 23, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can have it as an additional reference for quick help.

On a side note, I was trying to document the process of board port for other platforms but it seems like there is no easy way to do it. The above example I cited is an easy "dirty" way to port the CPU but not the "recommended" method as different maintainers use different processes. Which do you think makes sense to document the easy method or recommended method?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Which do you think makes sense to document the easy method or recommended method?

I never ported a CPU, so I can't really tell what methods are there and which are preferred by whom. Me in your place would go for what you think is the best method first and then ask for input by other maintainers in the PR.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay. That sounds good to me :)

@fjmolinas
Copy link
Contributor

ping @miri64 @akshaim

@miri64
Copy link
Member Author

miri64 commented May 4, 2021

ping @miri64 @akshaim

Still waiting for @akshaim's CPU porting guide.

@miri64
Copy link
Member Author

miri64 commented May 4, 2021

Alternatively, I could delete the reference to it for now.

@akshaim
Copy link
Member

akshaim commented May 4, 2021

Still waiting for @akshaim's CPU porting guide.

I think you can use the one I shared above for the CPUs from Silicon Labs. Regarding STM CPU's, I am currently porting one STM SOC but the port is not complete yet hence guide is incomplete. Regarding Atmel/Microchip CPU's I have not tried.

@MrKevinWeiss MrKevinWeiss added this to the Release 2021.07 milestone Jun 21, 2021
@MrKevinWeiss MrKevinWeiss removed this from the Release 2021.07 milestone Jul 15, 2021
@benpicco
Copy link
Contributor

Alternatively, I could delete the reference to it for now.

+1 let's move this forward.

@miri64
Copy link
Member Author

miri64 commented Sep 16, 2021

Done. I removed the porting-cpus.md altogether for now and added a "(TBD: provide guide)" to the graph. Maybe some of the /cpu contributors (@haukepetersen, @benpicco, @aabadie, @jnohlgard, @kaspar030, @basilfx, @kYc0o, ...) can pick up from that in a follow-up.

@benpicco
Copy link
Contributor

Please squash!

@miri64
Copy link
Member Author

miri64 commented Sep 16, 2021

Please squash!

Done

@benpicco benpicco added the CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR label Sep 16, 2021
@miri64 miri64 merged commit 77f7db1 into RIOT-OS:master Sep 16, 2021
@miri64 miri64 deleted the doc/enh/porting-graph branch September 16, 2021 16:53
@benpicco benpicco added this to the Release 2021.10 milestone Oct 20, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: doc Area: Documentation CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR CI: skip compile test If set, CI server will run only non-compile jobs, but no compile jobs or their dependent jobs Type: enhancement The issue suggests enhanceable parts / The PR enhances parts of the codebase / documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants