Export path configs as JSON

[pjreiniger]

This should make it easier for robot code to pull in the config file, rather than having to parse the CSV and deal with the TopLeft coordinate system (and having to compensate for the field height), rather than wpilibs BottomLeft coordinate system. This should make it much easier to use pathweaver to graphically draw the waypoints, but use your own code to generate the trajectory so you can choose between quintic/cubic, and specify different max velocity/acceleration for different paths.

I made two commits, a unit test that checks the loading before any changes were made, and another which makes the changes. This should give some piece of mind that everything else should still work despite the change in config structure.

I used the same names as present in the CSV file, but am willing to change. I also added a version indicator, so if something else comes along that wants to do a different schema (waymaker, adding constraints to the file, etc) the can bump the version and make another up-converter.

I tried to be as unintrusive as possble, and kept all the changes to the the import/export class.

[pjreiniger]

Ping

[shueja]

Hi, WayMaker dev here.

Looking over your schema, it seems to capture the proper data for PathWeaver trajectories. I will say that WayMaker will likely natively store paths in a significantly different schema: x, y, rotation, tangent length outwards from position. I am also floating the idea of storing interpolation type in the path, and even setting it per-segment (meaning one could switch from cubic to quintic without having to make two trajectories.

Additionally, to my knowledge, neither PathWeaver nor the trajectory generator actually support path reversal per-point, so it may be a chance for a breaking change to move that to the path level next to lengthUnit.

I can't speak from a position of having tested your modifications, I'm just offering my thoughts.

I was also going to comment on the introduction of Gson as a dependency, but it doesn't seem like there was another option already in use. The PathWeaver JSONs that represent the built trajectory run through TrajectoryUtil, which runs through JNI to what I can only assume uses wpi::json. However, that seems like a significant undertaking to use that JSON utility in this scenario rather than a new dependency.

[calcmogul]

The PathWeaver JSONs that represent the built trajectory run through TrajectoryUtil, which runs through JNI to what I can only assume uses wpi::json.

That's correct. We did that because Jackson was way slower at importing JSON-serialized trajectories than the C++ library. Imports were taking well over a few seconds. https://github.com/wpilibsuite/allwpilib/issues/3243

[pjreiniger]

x, y, rotation, tangent length

So is this just a decomposition of the tangent (X, Y)? If that is what the future wants I can make that change.

storing interpolation type in the path, and even setting it per-segment

V2 vs V1 (or backwards compatible V1.5, where a default is what pathweaver currently does). I mentioned that on CD, PathWeaver is the only option at the moment, and I'm for sure not making that update to PathWaver. With all the other changes that are about to happen, I view 2022 as the year of "big changes", a la completely re-writing the pathweaver configs to a different schema. and 2022 -> 2023 adding a new field and bumping a version number

reversal per-point, so it may be a chance for a breaking change

That is a good point, good opportunity to snuff it out. It isn't breaking if it is backwards compatible 😉

introduction of Gson as a dependency

Pathweaver already had the dependency. On the allwpilib / your team code there are other options. I think that some java library already has a dependency on jackson for json parsing, if you wanted to do it in the native language. Otherwise, I think it wouldn't be crazy to JNI out a C++ set of waypoints (or a TrajectoryConfig class in, Trajectory out). That is eventually what would have to be done on the allwpilib side anyway for WayMaker.

Jackson was way slower at importing JSON-serialized trajectories than the C++ library.

ACK. My counter argument would be that the path configs should be orders of magnitude smaller than the generated path. But, as mentioned above, lets do the otherside in the forward thinking way.

[calcmogul]

This is the work-in-progress schema for a WayMaker path:

{
  "name": string,
  "version": string,
  "units": string,
  "reversed": bool,
  "poses": [Pose2d, ...], # n poses
  "types": ["CubicHermite" | "QuinticHermite" | "Clothoid", ...], # n - 1 types
}

However, I'm concerned about changing the schema everything else uses until we're sure this is the format we want.

[shueja]

PathWeaver is the only option at the moment, and I'm for sure not making that update to PathWaver.

Wasn't expecting you to. (It also slipped my mind that we'd discussed that on CD already.)

So is this just a decomposition of the tangent (X, Y)? If that is what the future wants I can make that change.

Basically, yes. It's motivated by the way Pose2d stores its values.

Sidenote: I'm still not fully sure how PathWeaver handles the fixed theta with cubic splines. Is each fixed-theta point a start/endpoint for a separate cubic interpolation?

Side-side-note: is adjacent cubic segments from different interpolation sets something WayMaker should support in its segmented path type functionality? (don't answer that here)

However, I'm concerned about changing the schema everything else uses until we're sure this is the format we want.

I wouldn't worry about changing the PathWeaver schema beyond that reversal bool move. The PathWeaver required data and the WayMaker required data are different enough that it's best to just leave PathWeaver as a direct JSON analogue of the values in the CSV. There's basically just enough of an overlap to make an importer, but that's mostly it.

[jasondaming]

I like the schema calcmogul posted. If we convert to that from what we currently have I think the only thing lost is "fixed theta" which is really a display setting not a Pose/Path component. If they really want the fixed theta it can be re-enabled.

We also need the WPILib side of this that can read this file in. I had a script that parsed the csv.

What required data is missing to have PathWeaver use this same schema?

[shueja]

I'm not sure what you're asking about missing data. I will say, the schema above is an old concept. It shouldn't affect PathWeaver's capability to use that schema, but one new addition is drive direction set per-segment, allowing reversal in the middle of a path. In a PathWeaver-generated file, these flags would all be the same. I'm writing up the WayMaker design doc, and schema just moved to the top of the to-do.

[shueja]

Updated: Schema for WayMaker, with a special section for how it handles PathWeaver paths. Feedback?

https://github.com/shueja-personal/allwpilib/blob/waymaker/WAYMAKER.md#pathweaver-compatibility