refactor(preview): copy detector pointer payload#3
Merged
Conversation
Reviewer's guide (collapsed on small PRs)Reviewer's GuideRefactors VisionPreview to consume armor detector results as a direct pointer-based packet payload from the detector topic, and copies the underlying packet contents into its own queues to keep preview-side lifetimes independent of detector ownership. Sequence diagram for detector pointer payload handling in VisionPreviewsequenceDiagram
participant ArmorDetector
participant DetectorTopic
participant VisionPreview
participant DetectorCallback
participant PreviewQueue
ArmorDetector->>DetectorTopic: publish ArmorDetectionsMessage
DetectorTopic->>DetectorCallback: invoke with RawData
DetectorCallback->>DetectorCallback: cast data.addr_ to DetectorTopicMessage
DetectorCallback->>VisionPreview: PushDetector(**message)
VisionPreview->>PreviewQueue: copy ArmorDetectionsPacket contents into queue
PreviewQueue-->>VisionPreview: owns independent packet copy
ArmorDetector-->>DetectorTopic: manages original ArmorDetectionsPacket lifetime
Class diagram for VisionPreview detector message refactorclassDiagram
class VisionPreview {
+ImageFrame
+ImageTopic
+ImageData
+DetectorMessage
+DetectorTopicMessage
+DetectorMetrics
+Tracker
+TargetMessage
+PushDetector(DetectorMessage packet)
}
class ArmorDetectionsPacket {
}
class ArmorDetectionsMessage {
+ArmorDetectionsPacket* packet
+operator*() ArmorDetectionsPacket&
+operator!=(std::nullptr_t other) bool
}
class ArmorDetectorMetrics {
}
class ArmorTracker {
}
class CameraInfoV {
}
class SolveTrajectory_Target {
}
VisionPreview ..> ArmorDetectionsPacket : uses as DetectorMessage
VisionPreview ..> ArmorDetectionsMessage : uses as DetectorTopicMessage
VisionPreview ..> ArmorDetectorMetrics : uses
VisionPreview ..> ArmorTracker : uses
ArmorTracker ..> CameraInfoV : template parameter
VisionPreview ..> SolveTrajectory_Target : uses
ArmorDetectionsMessage --> ArmorDetectionsPacket : holds pointer
ArmorTracker <|-- ArmorTracker_CameraInfoV
class ArmorTracker_CameraInfoV {
}
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Hey - I've left some high level feedback:
- The use of
reinterpret_cast<DetectorTopicMessage*>(data.addr_)and double dereference**messagerelies on implicit assumptions about the raw payload layout; consider documenting or encapsulating this in a small helper (e.g.,DecodeDetectorPacket(data)) to make the ownership and pointer semantics explicit and easier to audit. - The null checks
message != nullptr && *message != nullptrimply thatDetectorTopicMessageis a nullable pointer-like type; consider using a stronger type (e.g., a wrapper or reference-like API) or adding an assertion/log when messages are unexpectedly null to surface upstream issues earlier.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The use of `reinterpret_cast<DetectorTopicMessage*>(data.addr_)` and double dereference `**message` relies on implicit assumptions about the raw payload layout; consider documenting or encapsulating this in a small helper (e.g., `DecodeDetectorPacket(data)`) to make the ownership and pointer semantics explicit and easier to audit.
- The null checks `message != nullptr && *message != nullptr` imply that `DetectorTopicMessage` is a nullable pointer-like type; consider using a stronger type (e.g., a wrapper or reference-like API) or adding an assertion/log when messages are unexpectedly null to surface upstream issues earlier.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
armor_detector/armors_resultas a direct detector-packet pointer payloadBoundary
Validation
ArmorDetectorandArmorTrackermaster merged pointer payload changes/tmp/direct_pointer_payload_20260502T180355ZSummary by Sourcery
Enhancements: