-
Notifications
You must be signed in to change notification settings - Fork 288
Advanced Controls Revamp #1821
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
Advanced Controls Revamp #1821
Conversation
Absolutely. The more pictures and visualizations, the better.
I'll look into only deploying the JS during build time, and I believe we have a script that minimizes at build time already. If not, it can be easily arranged. How large is the JavaScript? Another consider I have is mobile usage. Over half of our viewers are using mobile devices and we want to keep that in mind. Lastly, this is an excellent and must-needed contribution. I do want to make the glossary feature aware, and that it should be used quite liberally. We have hover tips enabled on glossary entries and it makes tracing the vocabulary so much easier. |
source/docs/software/advanced-controls/introduction/approaches_to_ctrl_sys_design.rst
Outdated
Show resolved
Hide resolved
source/docs/software/advanced-controls/introduction/approaches_to_ctrl_sys_design.rst
Outdated
Show resolved
Hide resolved
source/docs/software/advanced-controls/introduction/approaches_to_ctrl_sys_design.rst
Outdated
Show resolved
Hide resolved
source/docs/software/advanced-controls/introduction/approaches_to_ctrl_sys_design.rst
Outdated
Show resolved
Hide resolved
source/docs/software/advanced-controls/introduction/approaches_to_ctrl_sys_design.rst
Outdated
Show resolved
Hide resolved
source/docs/software/advanced-controls/introduction/tuning-bang-bang-controller.rst
Outdated
Show resolved
Hide resolved
Daltz333
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To keep things standard and consistent, we have the following style constraints.
- Articles with spaces in the name should be in the format
this-is-my-article. Instead ofthis_is_my_article - You should only have one blank line between paragraphs and sections. Remember that source readability is just as important as output.
- Section underlines go in the order of
=, -, ^, ~. If this you are using more than this, then the article should need to be restructured.
…_to_ctrl_sys_design.rst Co-authored-by: Dalton Smith <daltzsmith@gmail.com>
…_to_ctrl_sys_design.rst Co-authored-by: Dalton Smith <daltzsmith@gmail.com>
|
Thanks for all the early stuff - will work on incorporating it tonight and continuing to flesh out the actual wording on a lot of sections. From the technical side:
highcharts.js - ~300kb, already minified. |
|
Ok poking the group for a design review on the .js side before I get too far. 55aecc0 should have most of the functional content required. It's close to being split out how I would like, with much less code duplication, and better class names. Not perfect yet but directionally where I want to go. Additionally, conf.py has been updated to minify and concatenate all pid-tuning related javascript into a single file. However, as of that node, the files aren't getting concatenated in the right order, meaning the final output isn't actually valid javascript. As far as getting include order right, the options I have researched:
Thoughts? |
|
FWIW - |
…tch (#1) * add turret tuning article, restructure some other stuff to match * add section on when feedforward control is necessary * update structure (not simulations) of flywheel article * finish structural changes, propagate to vertical arm (minus some todos) * update index, links, add intro to feedforward, general improvements * try to restrict simulation to feedforward/feedback/both * finish flywheel article revamp * add click interaction to arm position demo * fix x axis value * Change vertical arm plant params to be reasonable * finish vertical arm * make vertical arm controller timing correct, record 10s * color consistency, plot labeling * add control effort to vertical arm visualization * fix various build woopsies * tune sizes * Add vector representation to flywheel visualization * add turret simulation * improve recommended turret kd gain * spelling and formatting
|
This seems good enough to merge now, to my eyes; it might need a couple of typographical changes per the Chief Delphi thread, but I don't think there are any major errors. I'm gonna go ahead and approve. |
Daltz333
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Additionally, I'd like to see most of the Wikipedia pages links/definitions recreated in the glossary section (for the tooltips), with the glossary definition linking to Wikipedia instead.
source/docs/software/advanced-controls/introduction/common-control-issues.rst
Outdated
Show resolved
Hide resolved
source/docs/software/advanced-controls/introduction/control-system-basics.rst
Outdated
Show resolved
Hide resolved
Daltz333
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think I'm seeing unrelated changes to the conf.py and GitHub actions workflow.
…gerth2_pid_tuning_updates # Conflicts: # source/docs/software/advanced-controls/introduction/common-control-issues.rst
|
To only include the js on pages that need, this code could be wrapped in a directive that does it when it's processed. |
* wip adding area for interactive walkthroughs * WIP adding javascript to plot things * more WIP getting interactive examples working * changed to light colors, added button controls * reorg and taking a few notes from the discord conversation * reworked controls to be text entry. Added flywheel visualization. * added time indication and other bug cleanup * ball injection checkbox * wip, more words * Update source/docs/software/advanced-controls/introduction/approaches_to_ctrl_sys_design.rst Co-authored-by: Dalton Smith <daltzsmith@gmail.com> * Update source/docs/software/advanced-controls/introduction/approaches_to_ctrl_sys_design.rst Co-authored-by: Dalton Smith <daltzsmith@gmail.com> * kebab case, not snake case. * WIP wording cleanups, missing wording in sections, section heading style * Missed some section headings * wip arm sim * arm wip - getting closer! * Graphical tweaks and adding cosin feed forward term * wip javascript refactor * wip automatically minifying a folder of js to a single included file * renames and bugfixes * visual apearance and link fixups * more wording * cleanup and methodlogy description * Update source/docs/software/advanced-controls/introduction/introduction-to-pid.rst Co-authored-by: Starlight220 <53231611+Starlight220@users.noreply.github.com> * Update source/docs/software/advanced-controls/introduction/tuning-pid-controller.rst Co-authored-by: Starlight220 <53231611+Starlight220@users.noreply.github.com> * Update source/index.rst Co-authored-by: Starlight220 <53231611+Starlight220@users.noreply.github.com> * Update source/docs/software/advanced-controls/introduction/introduction-to-pid.rst Co-authored-by: Starlight220 <53231611+Starlight220@users.noreply.github.com> * Update source/docs/software/advanced-controls/introduction/tuning-pid-controller.rst Co-authored-by: Starlight220 <53231611+Starlight220@users.noreply.github.com> * Update source/docs/software/advanced-controls/introduction/tuning-pid-controller.rst Co-authored-by: Starlight220 <53231611+Starlight220@users.noreply.github.com> * Update source/docs/software/advanced-controls/introduction/pid-video.rst Co-authored-by: Starlight220 <53231611+Starlight220@users.noreply.github.com> * Update source/docs/software/advanced-controls/introduction/tuning-pid-controller.rst Co-authored-by: Starlight220 <53231611+Starlight220@users.noreply.github.com> * Update source/docs/software/advanced-controls/introduction/tuning-pid-controller.rst Co-authored-by: Starlight220 <53231611+Starlight220@users.noreply.github.com> * Update source/docs/software/advanced-controls/introduction/tuning-pid-controller.rst Co-authored-by: Starlight220 <53231611+Starlight220@users.noreply.github.com> * resolving spelling errors and hyphenated feed-forward * resolved a bunch more conversations * Update source/docs/software/advanced-controls/introduction/tuning-pid-controller.rst Co-authored-by: Starlight220 <53231611+Starlight220@users.noreply.github.com> * resolved the remainder of Yotam's questions * wip covering some of oblarg's comments * All the comments have been accounted for! Huzzah! Now to make CI pass. * lint fixups pt 1 * lint check fixups pt 2 * black window reformat * attempt to fix sphinx build * missed js renames which broke things. * added ball to flywheel visualization * adjusted PID range * rewording and split voltage graph * cleaned up multi-chart formatting, added more plant/controller terminology * lint fixes * Axis title fixups * Reorganized tuning exercises to be by mechanism, not by control techinque * fixing redirect and a bit of wording * actually fixed redirects * redirects append * Delete black_windows.exe Accidentally added, this should not be present. * github comment fixup * moved inputs to use text input (not number) to inhibit undesired validation behavior * add turret tuning article, restructure some other stuff to match * add section on when feedforward control is necessary * update structure (not simulations) of flywheel article * finish structural changes, propagate to vertical arm (minus some todos) * update index, links, add intro to feedforward, general improvements * try to restrict simulation to feedforward/feedback/both * finish flywheel article revamp * add click interaction to arm position demo * fix x axis value * Change vertical arm plant params to be reasonable * finish vertical arm * make vertical arm controller timing correct, record 10s * color consistency, plot labeling * add control effort to vertical arm visualization * fix various build woopsies * tune sizes * Add vector representation to flywheel visualization * add turret simulation * improve recommended turret kd gain * spelling and formatting * [DRAFT] add turret tuning article, restructure some other stuff to match (#1) * add turret tuning article, restructure some other stuff to match * add section on when feedforward control is necessary * update structure (not simulations) of flywheel article * finish structural changes, propagate to vertical arm (minus some todos) * update index, links, add intro to feedforward, general improvements * try to restrict simulation to feedforward/feedback/both * finish flywheel article revamp * add click interaction to arm position demo * fix x axis value * Change vertical arm plant params to be reasonable * finish vertical arm * make vertical arm controller timing correct, record 10s * color consistency, plot labeling * add control effort to vertical arm visualization * fix various build woopsies * tune sizes * Add vector representation to flywheel visualization * add turret simulation * improve recommended turret kd gain * spelling and formatting * Apply suggestions from code review * File name cleanup and better comments/cleanup in conf.py * unbork. I am bad at merging. * finally fixed conf.py failing CI Co-authored-by: Dalton Smith <daltzsmith@gmail.com> Co-authored-by: Starlight220 <53231611+Starlight220@users.noreply.github.com> Co-authored-by: Oblarg <emichaelbarnett@gmail.com>
Since there's a decent amount of tearup here, starting a draft PR early to gather/document comments.
Core Goals
Technical Changes
Add sufficient js infrastructure to support interactive visualizations of sample mechanisms:
Source for the sim for the flywheel is here, and for the arm is here
To support this, we'd need:
Structural Changes
Status
May 5 2022 - Still very much in flight. Content is representative of the direction I want things to take, but far from complete or set in stone. Will update this PR when things are ready for detail review. In the mean time, comment away with concerns - it'll be easier to adjust course now, rather than in 5 months.See comments below for latest status.