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

Add LED patterns to LED documentation #2774

Open
wants to merge 25 commits into
base: main
Choose a base branch
from

Conversation

SamCarlberg
Copy link
Member

Resolves #2773

:language: c++
:lines: 15-20
:linenos:
:lineno-start: 15

.. image:: images/rainbow.gif
Copy link
Collaborator

Choose a reason for hiding this comment

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

I wonder if specifying the :width: will fix the pdf error

Copy link
Member Author

Choose a reason for hiding this comment

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

🤔 Hmm. I'll need to test that. I don't have LaTeX tools installed locally so it'll need to be on CI, which could be frustrating.

The style guide does explicitly call out gifs as being unacceptable for storage and accessibility concerns, but I don't see a way around it since the whole point of these docs are to demonstrate animations

Include screenshots/screen recordings of all examples

Examples mostly use the same discontinuous gradient for consistency and to make reversed and scrolling effects obvious
Should be 0.25 Hz, not 4 Hz
Move progress mask example to the mask example
@sciencewhiz
Copy link
Collaborator

this should remove the ignore from inspector.json

@sciencewhiz
Copy link
Collaborator

sciencewhiz commented Oct 12, 2024

Setting the width to 900 on the videos makes mobile scroll. Looks like the images get scaled down

@sciencewhiz
Copy link
Collaborator

If you put in a static png with the same name as the video files, I think that will fix the pdf build

@SamCarlberg
Copy link
Member Author

If you put in a static png with the same name as the video files, I think that will fix the pdf build

As in, images/scroll-relative.mp4 should have a corresponding images/scroll-relative.png file?

@SamCarlberg
Copy link
Member Author

only or ifconfig may also work, to conditionally render a video element or an image element based on the build target

@sciencewhiz
Copy link
Collaborator

sciencewhiz commented Oct 12, 2024

If you put in a static png with the same name as the video files, I think that will fix the pdf build

As in, images/scroll-relative.mp4 should have a corresponding images/scroll-relative.png file?

yes. That will work for image files, but I'm not sure if sphinx will automatically fall back from a video file to an image file

@sciencewhiz
Copy link
Collaborator

If you put in a static png with the same name as the video files, I think that will fix the pdf build

As in, images/scroll-relative.mp4 should have a corresponding images/scroll-relative.png file?

yes. That will work for image files, but I'm not sure if sphinx will automatically fall back from a video file to an image file

This won't work, looks like ..only would probably be best.

@sciencewhiz
Copy link
Collaborator

On chrome and edge, the videos don't show a play. Maybe because they are not tall enough? Works ok on Firefox

image

@SamCarlberg
Copy link
Member Author

I'm not sure, both firefox and chrome showed the controls overlaid on the videos in my testing. Annoyingly, the controls obscure the videos, and there doesn't seem to be any options for moving the (eg to be below the video). Maybe we could have some custom JS to make the controls appear on hover? https://stackoverflow.com/q/40284816

@SamCarlberg SamCarlberg marked this pull request as ready for review October 27, 2024 21:15
@TheTripleV
Copy link
Member

The video controls are there. The slider is the progress slider. If you click on the very very top few pixels of the video and press space, it will start to play and the controls will fade away.

Since this is looping, I would rather put in gifs for the website and png for the pdf.
APNG is support by all browsers https://caniuse.com/apng so maybe if you put in an apng, it will work on both web and pdf

@SamCarlberg
Copy link
Member Author

Since this is looping, I would rather put in gifs for the website and png for the pdf.

That was my preference too, but Daltz says it's an accessibility concern to have autoplaying gifs

@TheTripleV
Copy link
Member

I guess the fix then is making the videos a lot taller so the controls render properly

@sciencewhiz sciencewhiz mentioned this pull request Oct 28, 2024
@sciencewhiz
Copy link
Collaborator

I guess the fix then is making the videos a lot taller so the controls render properly

We might be able to do something like this, rather then make new videos: https://stackoverflow.com/a/65116261

Videos are paused by default

This avoids the issue of the controls overlapping and obscuring the animations
Render gifs everywhere else (PDFs should render just the first frame)
@zeroClearAmerican
Copy link

zeroClearAmerican commented Nov 1, 2024

A bit of feedback from our beta testing: LEDPattern examples sometimes refer to calling a function using the Units library, like scrollAtRelativeSpeed(Percent.per(Second).of(25)), however they don't specify that the Units library should be used. To a new student, this could be confusing because VSCode would report that an import for Percent would be missing when typed like the example.

Updating the example to include the Units reference before Percent, like scrollAtRelativeSpeed(Units.Percent.per(Second).of(25)) allows intellisense to understand and suggest the correct import and the user understands just by reading the example that Percent is part of the Units library.

@sciencewhiz
Copy link
Collaborator

this has conflicts now since some of the css was merged in the a-stop PR

@sciencewhiz
Copy link
Collaborator

\inspector fix all

Copy link
Contributor

github-actions bot commented Nov 2, 2024

Inspector Report


Up To Date

ALL @ <source/docs/contributing/frc-docs/style-guide.rst>
ALL @ <source/docs/romi-robot/programming-romi.rst>
ALL @ <source/docs/software/advanced-controls/controllers/profiled-pidcontroller.rst>
ALL @ <source/docs/software/advanced-controls/controllers/profiled-pidcontroller.rst>
ALL @ <source/docs/software/advanced-controls/controllers/trapezoidal-profiles.rst>
ALL @ <source/docs/software/advanced-controls/controllers/trapezoidal-profiles.rst>
ALL @ <source/docs/software/advanced-controls/state-space/state-space-flywheel-walkthrough.rst>
ALL @ <source/docs/software/advanced-controls/state-space/state-space-flywheel-walkthrough.rst>
ALL @ <source/docs/software/advanced-controls/state-space/state-space-observers.rst>
ALL @ <source/docs/software/advanced-controls/state-space/state-space-observers.rst>
ALL @ <source/docs/software/advanced-gradlerio/code-formatting.rst>
ALL @ <source/docs/software/advanced-gradlerio/profiling-with-visualvm.rst>
ALL @ <source/docs/software/basic-programming/java-gc.rst>
ALL @ <source/docs/software/basic-programming/robot-preferences.rst>
ALL @ <source/docs/software/basic-programming/robot-preferences.rst>
ALL @ <source/docs/software/can-devices/power-distribution-module.rst>
ALL @ <source/docs/software/can-devices/power-distribution-module.rst>
ALL @ <source/docs/software/commandbased/binding-commands-to-triggers.rst>
ALL @ <source/docs/software/commandbased/command-compositions.rst>
ALL @ <source/docs/software/commandbased/commands.rst>
ALL @ <source/docs/software/commandbased/cpp-command-discussion.rst>
ALL @ <source/docs/software/commandbased/pid-subsystems-commands.rst>
ALL @ <source/docs/software/commandbased/profile-subsystems-commands.rst>
ALL @ <source/docs/software/commandbased/structuring-command-based-project.rst>
ALL @ <source/docs/software/dashboards/glass/mech2d-widget.rst>
ALL @ <source/docs/software/hardware-apis/sensors/ultrasonics-software.rst>
ALL @ <source/docs/software/vision-processing/roborio/using-the-cameraserver-on-the-roborio.rst>
ALL @ <source/docs/software/vision-processing/roborio/using-the-cameraserver-on-the-roborio.rst>
ALL @ <source/docs/software/vscode-overview/creating-robot-program.rst>
ALL @ <source/docs/software/wpilib-tools/robot-simulation/physics-sim.rst>
ALL @ <source/docs/software/wpilib-tools/robot-simulation/unit-testing.rst>
ALL @ <source/docs/software/wpilib-tools/robot-simulation/unit-testing.rst>
ALL @ <source/docs/xrp-robot/programming-xrp.rst>
ALL @ <source/docs/zero-to-robot/step-4/creating-test-drivetrain-program-cpp-java-python.rst>
ALL @ <source/docs/zero-to-robot/step-4/creating-test-drivetrain-program-cpp-java-python.rst>

Outdated - Automatically Fixed

wpilibjExamples/src/main/java/edu/wpi/first/wpilibj/examples/addressableled/Robot.java#L32-L47 @ <source/docs/software/hardware-apis/misc/addressable-leds.rst:22>
wpilibcExamples/src/main/cpp/examples/AddressableLED/include/Robot.h#L12,L18-L27 @ <source/docs/software/hardware-apis/misc/addressable-leds.rst:31>
wpilibcExamples/src/main/cpp/examples/AddressableLED/cpp/Robot.cpp#L7-L13 @ <source/docs/software/hardware-apis/misc/addressable-leds.rst:37>
wpilibjExamples/src/main/java/edu/wpi/first/wpilibj/examples/addressableled/Robot.java#L21-L31 @ <source/docs/software/hardware-apis/misc/addressable-leds.rst:139>
wpilibcExamples/src/main/cpp/examples/AddressableLED/include/Robot.h#L27-L37 @ <source/docs/software/hardware-apis/misc/addressable-leds.rst:148>
wpilibjExamples/src/main/java/edu/wpi/first/wpilibj/examples/addressableled/Robot.java#L50-L56 @ <source/docs/software/hardware-apis/misc/addressable-leds.rst:161>
wpilibcExamples/src/main/cpp/examples/AddressableLED/cpp/Robot.cpp#L15-L20 @ <source/docs/software/hardware-apis/misc/addressable-leds.rst:170>

Invalid - Manual Intervention Needed

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.

Addressable LED Pattern documentation
4 participants