Conversation
Review: USD Best Practices AlignmentThis is a clean, well-scoped addition that addresses a real composition hazard. The core rule (clock is a simulator concern, not an asset concern) is sound and aligns well with USD's philosophy of separating structure from execution. The suggestions below are refinements, not blockers. Alignment highlights
Suggestions
Review performed against the NVIDIA Asset Structure Principles and the existing composition/namespace rules in this REP. |
| ### 2.8 Optical Frames | ||
| OpenUSD cameras natively face the -Z axis, whereas ROS optical frames (REP 103) must face +Z. To bridge this without opaque simulator-side rotations, authors must decouple the physical sensor from its optical interface. Authors must create a child UsdGeomXform (e.g., `camera_optical_frame`) rotated 180 degrees around its local X-axis. All RosTopicAPI and RosFrameAPI schemas must be applied exclusively to this optical frame, ensuring deterministic data orientation in RViz. | ||
|
|
||
| ### 2.9 Simulation Clock |
There was a problem hiding this comment.
This is a good idea and I would like to bring it further. How about we state it in a manner that also includes simulation interfaces? Perhaps "2.9 Prohibited interfaces"?
I would also like to keep it concise. How about this phrasing?
"Simulator-level interfaces are prohibited in assets to avoid clashes, including:
/clocktopic for simulation time.- Any interfaces included in the
simulation_interfacespackage (e.g. spawning, simulation control)"
There was a problem hiding this comment.
@asluk just noticed your review as well. I propose this concise phrasing and generalizing to "prohibited interfaces". One could further specify this to include per-node generated ROS interfaces (e.g. parameter services), but I believe that would be slightly too defensive.
Your suggestions about the clock topic are correct, however I would keep specification on how clock should be published out of scope of the REP (just that it's not assets purview).
There was a problem hiding this comment.
@adamdbrw I agree with your phrasing on prohibited interfaces.
I am wondering if its generally safe or good practice to directly list the exact packages that we prohibit in standard such as this? In case the package behaviour changes in the future? Should we leave the package name out of it? (Although one could argue that simulator specific unqiue interfaces should not be prohibited, so mentioning a standardized package like simulation_interefaces could be okay).
Which leads to my next question, I dont think theres a statement anywhere that says that interfaces can be defined within the USD asset, but if the simulator doesnt support it, the interface can simply be ignored with an optional warning? @adamdbrw thoughts on whether this needs to be added to the rep as well?
Added section describing not to apply clock topic api in robot assets in USD.