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 info on how DS/NetComm safety controls work (the sequel) #2308

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

Conversation

pjbuterbaugh
Copy link
Contributor

@pjbuterbaugh pjbuterbaugh commented Aug 8, 2023

Closes #2227.

Created a new PR after a bit of a mix-up on my end with branches.

@@ -0,0 +1,27 @@
Input/Output Safety Mechanisms Built into the 2015-2026 `FRC` Control System
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
Input/Output Safety Mechanisms Built into the 2015-2026 `FRC` Control System
Input/Output Safety Mechanisms Built into the `FRC` Control System

calling out the years is overly specfici and not really helpful. If the next control system has different safety mechanisms, the document would be updated.

@pjbuterbaugh pjbuterbaugh marked this pull request as ready for review August 10, 2023 14:02
Copy link
Member

@Daltz333 Daltz333 left a comment

Choose a reason for hiding this comment

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

Content is roughly fine, I'd have preferred some diagrams but I won't have that blocking merge. Remove any usages of "" as a delimiter, non-standard characters aren't translation friendly.

@@ -0,0 +1,27 @@
Input/Output Safety Mechanisms Built into the `FRC` Control System
============================================================================
Copy link
Member

Choose a reason for hiding this comment

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

Couple of issues here

  • Underline is too long (legal in rST spec, but against our styleguide)
  • You probably want ``FRC`` instead of FRC
  • The title is too long, I think "Control System" and "FRC" is redundant


Robot side: there are multiple hardware and software components involved. Outputs of the RoboRIO (e.g. :term:`PWM` s) are controlled by the :term:`FPGA` hardware. :term:`NetComm` is a software daemon that talks to the DS, the FPGA, and the user program. Inside of the user process \(the team\’s robot program\), there\’s a NetComm :term:`DLL` component that talks to the FPGA, CAN, and the NetComm daemon. And of course there are CAN motor controllers on the CAN bus.

- The FPGA has a system :term:`watchdog`. This watchdog will time out and force a “disable” of PWM motor outputs if NetComm hasn\’t told it it\’s received an enable packet in the last 125 `ms`.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
- The FPGA has a system :term:`watchdog`. This watchdog will time out and force a “disable” of PWM motor outputs if NetComm hasn\’t told it it\’s received an enable packet in the last 125 `ms`.
- The FPGA has a system :term:`watchdog`. This watchdog will time out and force a “disable” of PWM motor outputs if NetComm hasn\’t told it it\’s received an enable packet in the last 125 `ms`.

Don't use non-UTF8 quotations. Replace “ with ". This helps our translators. Additionally, replace the non-UTF8 apostrophe with '

Robot side: there are multiple hardware and software components involved. Outputs of the RoboRIO (e.g. :term:`PWM` s) are controlled by the :term:`FPGA` hardware. :term:`NetComm` is a software daemon that talks to the DS, the FPGA, and the user program. Inside of the user process \(the team\’s robot program\), there\’s a NetComm :term:`DLL` component that talks to the FPGA, CAN, and the NetComm daemon. And of course there are CAN motor controllers on the CAN bus.

- The FPGA has a system :term:`watchdog`. This watchdog will time out and force a “disable” of PWM motor outputs if NetComm hasn\’t told it it\’s received an enable packet in the last 125 `ms`.
- The NetComm DLL in the user process will send a disable broadcast message on the CAN network and then stop sending keep-alive CAN messages after the disabled system watchdog signal is read back from the FPGA \(this is checked on a 20 `ms` loop\). REV pneumatics and motor controllers will stop immediately upon receipt of the disable broadcast. They also stop if no keep-alive is received for 100 `ms` \(pneumatics\) or 220 `ms` \(motor controllers\).

Choose a reason for hiding this comment

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

Our specific timeout values are really just implementation details. It's one thing to share them for a Chief Delphi post that isn't expected to be evergreen, and another thing to put them somewhere that's expected to be an up-to-date resource. Here's my suggestion for a more generic replacement.

Suggested change
- The NetComm DLL in the user process will send a disable broadcast message on the CAN network and then stop sending keep-alive CAN messages after the disabled system watchdog signal is read back from the FPGA \(this is checked on a 20 `ms` loop\). REV pneumatics and motor controllers will stop immediately upon receipt of the disable broadcast. They also stop if no keep-alive is received for 100 `ms` \(pneumatics\) or 220 `ms` \(motor controllers\).
- The NetComm DLL in the user process will send a disable broadcast message on the CAN network and then stop sending keep-alive CAN messages after the disabled system watchdog signal is read back from the FPGA \(this is checked on a 20 `ms` loop\). REV pneumatics and motor controllers will stop immediately upon receipt of the disable broadcast. They also stop if no keep-alive is received after a very brief timeout period.

Choose a reason for hiding this comment

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

Additionally, there is incorrect information about how the roboRIO's software behaves in this paragraph. While the roboRIO is booted, it never stops sending Universal Heartbeat messages. When the robot is stopped, the roboRIO merely changes the WatchdogEnabled field from a 1 to a 0, but it keeps sending out the heartbeat.

This paragraph should also probably use the official "heartbeat" term consistently, rather than "keep-alive", which I think more strongly implies that receipt of the frame itself indicates an enabled status, rather than the truth, which is that it merely indicates the presence of a roboRIO, and you have to look at a specific byte to check the enabled status.

Copy link
Contributor

Choose a reason for hiding this comment

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

Not to block the PR, but I think it would be nice to have the timeout value(s) documented. Even if implementations will vary and even change, would it be fair to document minimum and maximum timeouts that all implementations fall inside of?

Copy link
Member

@rzblue rzblue Mar 7, 2024

Choose a reason for hiding this comment

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

I think its important to have those timeouts (or at least an upper bound) documented. @Kevin-OConnor maybe you can comment on what FIRST's expectation of motor controllers' timeout period is?

Copy link
Contributor

Choose a reason for hiding this comment

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

It seems odd that this specifically calls out what REVs devices do while not saying anything about other vendors. This should probably be genericized and use language similar to the "brief timeout period" Noah has suggested.

Copy link
Member

@rzblue rzblue left a comment

Choose a reason for hiding this comment

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

Ideally this should be updated to include the new DS safety timeouts, since those aren't documented anywhere in detail

- When NetComm receives a control packet from the DS with enable set to true, it will immediately enable motors \(and restart the FPGA watchdog timer\).
- A count of watchdog expiration events is sent by NetComm to the DS, so this data is in the DS log.

Software Side:
Copy link
Member

Choose a reason for hiding this comment

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

This should be "Driver Station side" or similar

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.

Add info on how DS/NetComm safety controls work
7 participants