wpilibsuite / shuffleboard

A modern dashboard for FRC
Other
79 stars 84 forks source link

Correct FieldData de/serialization #798

Closed Starlight220 closed 3 months ago

Starlight220 commented 4 months ago

The first commit fixes #797 The second commit is a bug I saw and decided to fix on the way


The fix here is to explicitly check for the type being double[] and warning+dropping if not, rather than unsafely casting which leads to CCEs like #797 in cases where there's unrelated data in that table.

PeterJohnson commented 4 months ago

Note that next year we will probably change things to publish a Pose2d struct instead.

Starlight220 commented 4 months ago

That can easily and non-breakingly be done by adding another else if clause to the deserialization. And if we're already changing the schema, it should probably include an objects: string[] entry at the root table so the impl doesn't need to scan everything.

Starlight220 commented 3 months ago

I have reproduced the issue with main, and confirmed that the issue does not reproduce with a build from this branch. My test code was the ramsetecontroller example, with the following modification:

diff --git a/wpilibjExamples/src/main/java/edu/wpi/first/wpilibj/examples/ramsetecontroller/Robot.java b/wpilibjExamples/src/main/java/edu/wpi/first/wpilibj/examples/ramsetecontroller/Robot.java
index ef73d106d..8a1a1bcb7 100644
--- a/wpilibjExamples/src/main/java/edu/wpi/first/wpilibj/examples/ramsetecontroller/Robot.java
+++ b/wpilibjExamples/src/main/java/edu/wpi/first/wpilibj/examples/ramsetecontroller/Robot.java
@@ -53,7 +53,8 @@ public class Robot extends TimedRobot {

     // Create and push Field2d to SmartDashboard.
     m_field = new Field2d();
-    SmartDashboard.putData(m_field);
+    SmartDashboard.putData("myfield", m_field);
+    SmartDashboard.putNumber("myfield/spam", 42);

     // Push the trajectory to Field2d.
     m_field.getObject("traj").setTrajectory(m_trajectory);