Skip to content

Conversation

Copy link
Contributor

Copilot AI commented Nov 28, 2025

Addresses review feedback on #542 to remove defensive fallbacks and verify pose detection math.

Changes

Removed fallbacks:

  • Direct import of pupil_apriltags (raises ImportError if missing)
  • load_camera_intrinsics() propagates FileNotFoundError
  • detect_apriltag_pupil() raises FileNotFoundError for missing images
  • Removed pd.Timestamp.now() fallback in JSON export

Explicit tag family:

  • Added --tag-family CLI argument (default: tag36h11)
  • Removed multi-family fallback loop that tried 4 families sequentially
python detect_apriltag.py img.png --tag-family tag36h11

Math verification:
Cross-checked against pupil-apriltags docs — implementation is correct:

  • Camera params: [fx, fy, cx, cy] from intrinsic matrix
  • Tag size in meters
  • Euler angles via R.from_matrix(pose_R).as_euler('xyz', degrees=True)

✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.

Copilot AI self-assigned this Nov 28, 2025
@sgbaird
Copy link
Member

sgbaird commented Nov 28, 2025

@copilot also, you shouldn't archive any of those files.

Copilot AI changed the title [WIP] Fix fallbacks and improve math in Apriltag calibration Remove fallbacks and add explicit tag family parameter in AprilTag detection Nov 28, 2025
Copilot AI requested a review from sgbaird November 28, 2025 03:04
Copy link
Contributor Author

Copilot AI commented Nov 28, 2025

@copilot also, you shouldn't archive any of those files.

The archive folder and file renames appear to be from the parent PR #542 (commit 00c0117), not from my changes in this sub-PR. My commits only modified CHANGELOG.md and detect_apriltag.py to remove fallbacks and add the --tag-family parameter. Should I revert the archive changes in the parent PR #542?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants