Add AprilTag Tag Info to Field JSON about allwpilib HOT 6 OPEN

We only support tags the official FRC (and maybe eventually FTC) field supports. Furthermore, the AprilTag class only contains ID and pose because that's all robot code cares about; I don't see why tag family and tag size need to be added.

Since robot code would never use this, is this for printing tags instead? In general, we don't add features unless they have a demonstrated concrete use case.

from allwpilib.

I mean so I can make a custom field. Obviously, I could provide my custom field JSON to the parser, and it could handle tag locations only. (And yes, I understand that WpiLib would never check in my custom field.)

These 2 fields to add are currently being used in the WpiLib AprilTag robot example, so I don't see how you can say they don't needed to be added. This would mean the robot program doesn't have to keep these field constants, as the JSON file could have them.

1. detector.addFamily("tag36h11", 1); - This could use the information from the JSON
2. var poseEstConfig = new AprilTagPoseEstimator.Config(0.1651, [camera focal info]); - This has the tag size in meters, which could come from the JSON file too, 0.1651m = 6.5".

I guess this brings up a problem in the example, how to handle multiple tag sizes on the field (like FTC did this season), as you would need 2 Pose estimators...

from allwpilib.

That example shows how to detect AprilTags with a USB camera plugged into the roboRIO (not recommended, but it's there for low-resource teams). It only produces camera-to-tag transformations, so it wants the tag family and tag size, but it doesn't care what the tag IDs, field-relative tag locations, or field dimensions are.

The AprilTag field layout JSON is intended to be used with the AprilTagFieldLayout class to provide pairs of tag IDs and field-relative poses. The field dimensions are included in the JSON to support the default red and blue alliance field origins. Here's some example usage with a DifferentialDrivePoseEstimator that assumes one tag. It wants the tag IDs and field-relative tag locations, but it doesn't care what the tag families or tag sizes are.

I suppose in general, we assume the detector is on a different processor than the consumer, and those programs have completely different information needs.

I guess this brings up a problem in the example, how to handle multiple tag sizes on the field (like FTC did this season), as you would need 2 Pose estimators...

You'd need a different AprilTagDetector for each tag family and a different AprilTagPoseEstimator for each tag size. Annotating each of the JSON's tags with family and size doesn't really help either class; it's up to the user to look up what kinds of tags the field has and make detector/estimator instances for them.

from allwpilib.

I guess you said my whole point in your last comment:

it's up to the user to look up what kinds of tags the field has and make detector/estimator instances for them.

Why? These are field constants, not robot constants, and I feel the field JSON file should describe these constants.

Anyways, I made a prototype of what changes I would propose. It's not the cleanest code, but enough to prove the point and how it could be useful to someone else. See main...scarmain:allwpilib:apriltag-sizes. There definitely could be some work on making the change more transparent with constructors that don't require these new fields. I updated the AprilTag example with how it could be used. I know it would need a C++ implementation too.

I could see people getting a benefit out of these functions. To my knowledge, if you have extra fields in a JSON file that the code doesn't care about, it doesn't hurt anything. But if multiple people of the WpiLib team don't see any value in this, I can stop the work on this change.

from allwpilib.

That change makes writing a detector (a rare event) more general at the expense of more unnecessary serialization and network I/O overhead for everyone else in the much more common case of just consuming tag poses over NetworkTables. Everyone will have to serialize another AprilTag field even though no robot code will actually use it. That change didn't save the detector writer any work either.

from allwpilib.

I don't understand the comment about unnecessary network I/O, Where would this table go to impact network I/O? It should stay local to the cpu calculating it. Are you worried about the one time you upload it?

I slightly see the extra work deserializing it, but if you are doing this more than once a power cycle during init, you are doing this wrong. This change would be a couple microseconds once, and should be very negligible.

I will say this too. While I agree, not a lot of people are using the WpiLib AprilTag detection in the RoboRio, these AprilTag libraries ARE being used in PhotonVision. See https://github.com/PhotonVision/photonvision/blob/master/photon-core/src/main/java/org/photonvision/vision/pipeline/AprilTagPipeline.java. You can see they use the WpiLib field JSON too, here: https://github.com/PhotonVision/photonvision/blob/010688006a53d9ce06ea6d02c71ba02003717a5b/photon-core/src/main/java/org/photonvision/common/configuration/SqlConfigProvider.java#L290

from allwpilib.