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

Adding Stabilizer User Manual #401

Merged
merged 22 commits into from
Jul 16, 2021
Merged

Adding Stabilizer User Manual #401

merged 22 commits into from
Jul 16, 2021

Conversation

ryan-summers
Copy link
Member

@ryan-summers ryan-summers commented Jul 9, 2021

This PR adds a user manual to Stabilizer using Jekyll + Github Pages.

You can see a sample of this site on http://quartiq.de/stabilizer

The page is served from the pages branch and a workflow has been set up to automatically copy and release docs on that branch when master is updated.

Existing docs include:

  • Flashing, building, debugging firmware
  • Setting up MQTT
  • Information on livestreaming
  • Documentation of run-time settings.

This is intended as an initial stroke. With the framework in place, we can now easily add further docs via markdown and have them easily served to the website.

@jordens
Copy link
Member

jordens commented Jul 9, 2021

A few items:

  • Why can't we just use cargo doc?
  • I can't seem to access the docs generated by cargo doc. Where do they appear and where are they linked?
  • the readme should be reduced significantly and point to the docs.

@ryan-summers
Copy link
Member Author

ryan-summers commented Jul 10, 2021

A few items:

  • Why can't we just use cargo doc?

I originally investigated putting all of the docs into stabilizer/src/lib.rs and using cargo doc. While it does work, I wasn't really happy with the granularity it provided - it felt like there were a lot of docs jammed on one page and it didn't feel clean to me. That's not to say it's impossible, and going towards cargo-doc only is an approach that we can definitely still take if we'd like. I went with the above approach largely because I felt it was nice and clean to separate some of the docs into separate pages to make it easier to navigate and digest.

There are definite downsides to this though, such as the docs living in separate files from the source code. This risks the docs becoming outdated when source code is updated. That being said, there's also a fair number of docs that are totally unrelated to code, such as setting up MQTT, toolchains, flashing, debugging, etc.

This is why I originally went with the combined approach of cargo-doc + Github pages / Jekyll. Definitely open to feedback - it's easy to move things around and reorganize.

  • I can't seem to access the docs generated by cargo doc. Where do they appear and where are they linked?

This is an oversight on my part. The docs are uploaded to firmware/<app>/index.html from the website (under http://quartiq.de/stabilizer/pages/networking/mqtt.html#settings-configuration). They're only linked from MQTT -> settings -> app specific settings, but I personally think we should add them as a direct link to the menu on the left for ease of access.

  • the readme should be reduced significantly and point to the docs.

Agreed - just haven't gotten to this yet.

@jordens
Copy link
Member

jordens commented Jul 11, 2021

Maybe use dummy mods to structure instead of putting it all into the lib doc?
Also with 1.54 we can use external docs as below. I'm fine with requiring beta/nightly to build the docs for now. It would be nice to avoid the additional web site tooling, style and infra if we already have cargo doc. That also helps with consistency and dangling link avoidance.

#[doc = include_str!("my_doc.md")]
 struct S;

docs/index.md Outdated Show resolved Hide resolved
docs/index.md Outdated Show resolved Hide resolved
docs/pages/firmware/customization.md Outdated Show resolved Hide resolved
docs/pages/firmware/customization.md Outdated Show resolved Hide resolved
docs/pages/firmware/customization.md Outdated Show resolved Hide resolved
docs/pages/networking/livestreaming.md Outdated Show resolved Hide resolved
docs/index.md Outdated Show resolved Hide resolved
docs/pages/firmware/index.md Outdated Show resolved Hide resolved
docs/pages/networking/mqtt.md Outdated Show resolved Hide resolved
docs/pages/networking/livestreaming.md Outdated Show resolved Hide resolved
@jordens
Copy link
Member

jordens commented Jul 14, 2021

Also https://

.github/workflows/release-docs.yml Outdated Show resolved Hide resolved
.gitignore Outdated Show resolved Hide resolved
docs/pages/getting-started.md Outdated Show resolved Hide resolved
docs/pages/getting-started.md Outdated Show resolved Hide resolved
docs/pages/networking/run-time-settings.md Outdated Show resolved Hide resolved
README.md Outdated
@@ -17,7 +17,7 @@ Several applications are provides by default:

### Dual-IIR

![Flow diagram](stabilizer_pid.svg)
![Flow diagram](docs/assets/stabilizer_pid.svg)
Copy link
Member Author

Choose a reason for hiding this comment

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

This diagram is outdated and inaccurate. Docs in this README should now point to the stabilizer user manual site.

Copy link
Member

Choose a reason for hiding this comment

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

@SingularitySurfer could you spin a new diagram (sample rate 781 kHz) ? The existing one's source is the SVG in the repo if you need it. I used draw.io back then.

Regarding including/showing images in cargo docs, it works just fine with relative links for all "local" crates (which is what we care about here) and with absolute links for everything else.

https://docs.rs/ndarray/0.11.2/ndarray/type.ArrayView.html#method.split_at
or
https://users.rust-lang.org/t/include-images-in-rustdoc-output/3487/4

docs/index.md Outdated Show resolved Hide resolved
@ryan-summers
Copy link
Member Author

@jordens It looks like we can't generate docs for dsp because one of the dev-dependencies uses std. I opened an issue for it in rust-lang/cargo#9690. It looks like one of our tests is using functions from ndarray that require std (dsp/src/rpll.rs:177 and 179)

@jordens
Copy link
Member

jordens commented Jul 15, 2021

Ah. The pages branch should only ever contain the single HEAD commit, no history. Otherwise this will blow up. There is some git option for that IIRC.

@ryan-summers
Copy link
Member Author

@jordens I've gotten htmlproofer running via rake test in the docs folder to check for correct linkage in the website

Copy link
Member

@jordens jordens left a comment

Choose a reason for hiding this comment

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

A couple minor things. Let's land and then tweak the rest later.

docs/README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
docs/index.md Outdated Show resolved Hide resolved
docs/index.md Show resolved Hide resolved
docs/index.md Show resolved Hide resolved
dsp/src/iir.rs Outdated Show resolved Hide resolved
src/configuration.rs Outdated Show resolved Hide resolved
src/bin/dual-iir.rs Show resolved Hide resolved
src/net/data_stream.rs Show resolved Hide resolved
src/net/telemetry.rs Show resolved Hide resolved
docs/pages/usage.md Outdated Show resolved Hide resolved
Co-authored-by: Robert Jördens <rj@quartiq.de>
@jordens jordens self-requested a review July 16, 2021 12:44
Copy link
Member

@jordens jordens left a comment

Choose a reason for hiding this comment

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

Let's roll.

@jordens jordens mentioned this pull request Jul 16, 2021
3 tasks
@ryan-summers ryan-summers merged commit 39f4e27 into master Jul 16, 2021
@bors bors bot deleted the feature/pages-docs branch July 16, 2021 13:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants