search

wpilibsuite / allwpilib

Official Repository of WPILibJ and WPILibC
https://wpilib.org/
Other
1.05k stars 612 forks source link

Actuators controllable via dashboard regardless of LiveWindow #5162

Open Starlight220 opened 1 year ago

Starlight220 commented 1 year ago

Describe the bug Motors etc are controllable from the dashboard even with LiveWindow disabled (ie in regular teleop). NOTE: I've only reproduced this in sim; haven't tried on a real robot.

To Reproduce Steps to reproduce the behavior:

  1. Publish a PWMSparkMax object to the dashboard using SmartDashboard or Shuffleboard.
  2. Enable teleop in the SimGUI (haven't tested with a real DS).
  3. Modify the NT value for the motor controller's commanded duty cycle. I've tested with the SimGUI and Glass.
  4. Observe that the actual commanded value in the hardware view changes to the one set from NT.

Much simpler, this can be seen that the .controllable topic for anything put to the dashboard is true. This is likely the source of the bug.

Expected behavior .controllable is false and values from network are ignored, except for when LW control is active.

Additional context @Oblarg seems that LiveWindow isn't obviously working in any case.

Starlight220 commented 1 year ago

5163 adds a test that proves this issue.

I'm working on a fix, it may be rather involved.

Starlight220 commented 1 year ago

So something I missed is that LiveWindow control isn't in-place: it publishes everything separately under the LiveWindow table, and that's where the controllable data is -- the original publishing (via Shuffleboard, SmartDashboard, etc) is untouched.

Do we want to keep this behavior? Or would it be better to use the in-place publishing for LW control?

PeterJohnson commented 1 year ago

Yes the code for this currently ignores the actuator setting. It’s a bug, but we aren’t going to touch this code during the season.

It needs to be a separate table since it’s auto-generated from objects rather than dependent on user code to putData to SmartDashboard.

Starlight220 commented 1 year ago

I'm not talking about the .actuator setting -- that works correctly, though I'm not sure what uses it. I'm talking about the .controllable setting, which I think is used from Shuffleboard etc., and in this case is always true.

As for the LiveWindow table, I think that depends on how we want LiveWindow to be moving forward. I think that some teams would want LW to make stuff controllable in-place during test-mode.

This may also be what happened to commands in beta this year; I'm not sure they were supposed to be controllable outside of test mode. If they're always controllable, it raises a question for NetworkButton.