[wpilibj] Watchdog Alert Linking #7160
Conversation
|
I had a more performant way of creating the new alert text but removed it in favor of |
|
This can work hand in hand with #7099 If both are merged there should be a way to make the default watchdogs not print to the console so you can have NT only timing diagnostics. |
| alert.m_active = true; | ||
| alert.m_activeStartTime = nowSeconds; | ||
| alert.m_text = String.format("%s [x%d]", m_originalAlertText, m_overruns); |
There was a problem hiding this comment.
Alert already has methods for setting the alert and updating the text, use those.
Why is the start time when the watchdog was last fed? The problem started when the watchdog ran out, not when it was last fed.
There was a problem hiding this comment.
Now is not when last fed, its when the notifier unblocks.
I needed a way to get the alerts original text. And the alternative to directly accessing the m_active and m_activeStartTime is pulling the alert low then high every cycle.
I thought package private would just be easier, along with unit tests.
There was a problem hiding this comment.
Now is not when last fed, it's when the notifier unblocks.
Yep, sorry.
Why does start time need to be reset?
To get the original text you can add a getter to Alert. In general, exposing internal data for the purpose of unit testing is a code smell.
| return; | ||
| } | ||
| Alert alert = m_alert.get(); | ||
| alert.m_active = true; |
There was a problem hiding this comment.
That doesn't matter
There was a problem hiding this comment.
The alert will always be active after the first time the watchdog times out. This makes it useless.
| private double m_lastTimeoutPrintSeconds; | ||
|
|
||
| int m_overruns; |
| m_timeoutSeconds = timeoutSeconds; | ||
| m_callback = d -> callback.run(); | ||
| m_tracer = new Tracer(); | ||
| } | ||
|
|
||
| /** | ||
| * Watchdog constructor. | ||
| * | ||
| * @param timeoutSeconds The watchdog's timeout in seconds with microsecond resolution. | ||
| * @param callback This function is called with the time in seconds since the watchdog was last | ||
| * fed when the timeout expires. | ||
| */ | ||
| public Watchdog(double timeoutSeconds, DoubleConsumer callback) { |
| // Set expiration flag before calling the callback so any | ||
| // manipulation of the flag in the callback (e.g., calling | ||
| // Disable()) isn't clobbered. | ||
| watchdog.m_isExpired = true; | ||
|
|
||
| m_queueMutex.unlock(); | ||
| watchdog.m_callback.run(); | ||
| watchdog.m_callback.accept(now - watchdog.m_startTimeSeconds); |
|
We also need to be cautious of where we are tying APIs to Alert, as it is an incubating api that will likely change in the short term. I think this is probably fine though. |
|
I think it's really important that any WPILib/vendor logic tied to |
|
This also will need a c++ version eventually |
Allows linking an
Alertto aWatchdog.A linked alert will be activated every time the watchdog times out. On timeout the activation time will be reset and the text of the alert will be updated to
"<alert-text> [xN]"whereNis the number of overruns that have occurred.