Skip to content

ArUco od ID 1 do ID 99 oraz tabliczka#74

Open
PFlak wants to merge 1 commit into
developfrom
ArUco
Open

ArUco od ID 1 do ID 99 oraz tabliczka#74
PFlak wants to merge 1 commit into
developfrom
ArUco

Conversation

@PFlak

@PFlak PFlak commented May 4, 2026

Copy link
Copy Markdown
Member

No description provided.

Dodaję wygenerowane pody ArUco do wrzucenia na tabliczkę oznaczoną "aurco-1.obj".
Wszystkie te pliki PNG mogą posłużyć jako tekstury na tę tabliczkę.
Tabliczka zrobiona zgodnie z wymogami w regulaminie TAC Challange.

Nie jestem pewien, czy aurco-1 obj aby współgrał z teksturą, nie potrzebuje tekstury o nazwie "aruco-1", bo plik .obj szuka tekstury po nazwie (przynajmniej u mnie lokalnie), jeśli się pojawią z tym problemy, zmiana nazwy konkretnego pliku powinno rozwiazać problem.
@github-actions

github-actions Bot commented May 4, 2026

Copy link
Copy Markdown

🤖 Crush Code Review Bot Status

Status: success

🤖 Crush Code Review

Overall Assessment:
This PR adds ArUco marker files (IDs 1-99 as PNG textures and one 3D mesh file) to support visual marker-based localization for the ROV simulation. The changes are purely additive asset additions with no code modifications. While the intent is clear for competition requirements, there are several organizational and technical issues that need addressing.

🔴 Critical Issues (Must Fix)

ARUCO/aurco-1.obj:3: Missing .mtl material file - The OBJ file references aurco-1.mtl but this file is not present in the PR, which will cause rendering failures in Stonefish and other 3D viewers.
ARUCO/aurco-1.obj:3: Typo in filename - The .obj file references "aurco-1.mtl" but the actual file is named "aruco-marker-ID=*.png" (note the typo: "aurco" vs "aruco"). This mismatch will prevent texture loading.
ARUCO/: Wrong directory location - ArUco markers are placed at workspace root instead of within the rov_stonefish/data/ directory where other simulation assets (cliff, valve_panel, etc.) are stored. This breaks project organization patterns.
ARUCO/: Missing package integration - No CMakeLists.txt or package.xml modifications in rov_stonefish to install these assets, meaning they won't be available in the workspace installation directory at runtime.

🟡 Important Issues (Should Fix)

ARUCO/aruco-marker-ID=*.png: Inconsistent naming - Files use format aruco-marker-ID=N.png instead of standard DICT_4X4_50_ID=N.png or aruco_ID=N.png, which may require custom parsing in detection code.
ARUCO/: No documentation - No README or documentation explaining the ArUco dictionary type (4x4, 5x5, etc.), marker size in meters, or intended usage in TAC Challenge scenarios.
ARUCO/aurco-1.obj: Hardcoded texture reference - The OBJ file assumes a specific texture name but doesn't specify which of the 99 PNG files should be used, making the mesh unusable without manual configuration.
ARUCO/: Large file size impact - Adding 99 PNG files (~14.5KB each) increases repository size by ~1.4MB, which may slow down clone operations for developers.

🔵 Minor Suggestions (Nice to Have)

ARUCO/: Consider compressing assets - PNG format is appropriate but verify if lossless compression could reduce file sizes without affecting detection accuracy.
ARUCO/: Add validation script - Consider adding a Python script to verify marker detectability using OpenCV's ArUco module before integration.
ARUCO/: Version markers - Consider adding metadata or documentation about which ArUco dictionary these markers belong to (e.g., DICT_4X4_100, DICT_6X6_250).

Best Practice Suggestions:

  • Are these markers standardized for the competition or custom-generated?
  • Should there be a separate launch file or scenario file that demonstrates marker usage?
  • Are there specific pose/orientation requirements for marker placement in the simulation scene?

Positive Feedback:

  • 3D mesh model (aurco-1.obj) provides a proper physical representation for simulation
  • Clean PNG format suitable for texture mapping and ArUco detection
  • No code changes means low risk of introducing bugs

This bot uses Crush AI to analyze your code changes and provide suggestions.

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