Register brain_box_msgs/Hist so the package generates and ships it - #80
Open
eastagiletracker wants to merge 1 commit into
Open
Register brain_box_msgs/Hist so the package generates and ships it#80eastagiletracker wants to merge 1 commit into
eastagiletracker wants to merge 1 commit into
Conversation
Hist.msg exists under msg/ but was never listed in rosidl_generate_interfaces(), so the Hist interface is never generated and downstream packages cannot use brain_box_msgs/msg/Hist. Register msg/Hist.msg and qualify its header field to std_msgs/Header (it was a bare "Header", which rosidl resolves to the nonexistent brain_box_msgs/Header). This matches its sibling *Hist messages (CeilingHist, EdgeHist, ...) and the other 94 headered messages in the package.
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.
This PR proposes registering
brain_box_msgs/Histin the package's message manifest so it is generated and shipped, and qualifying its header field tostd_msgs/Header. We include this PR work along with a full history of your repo at https://eastagiletracker.com/projects/254. You can sign in with your GitHub ID to claim ownership of the project.What this fixes
brain_box_msgs/msg/Hist.msgexists in the tree and belongs to the same histogram family asCeilingHist,EdgeHist,CorrFrontHist,CorrSidesHistandPanelHist, but it was never added torosidl_generate_interfaces()inbrain_box_msgs/CMakeLists.txt. Because that macro generates exactly the interfaces it is given,Histis never generated and no downstream package can usebrain_box_msgs/msg/Hist.There is a second, coupled problem:
Hist.msgdeclares a bareHeader headerfield. Unlike ROS 1, ROS 2 does not special-caseHeader;rosidlresolves an unqualified type against the current package, soHeaderbecomesbrain_box_msgs/Header— a message that does not exist. Every other headered message in the package (its*Histsiblings and 94 others) already usesstd_msgs/Header, so simply listingHist.msgas-is would break message generation. This PR does both halves: it registersmsg/Hist.msgand qualifies the field tostd_msgs/Header.Reproducing on the current
ros2HEADInterface file present on disk but missing from the manifest:
The lone bare header (all other definitions already qualify it):
I also ran the ROS 2 message parser (
rosidl_adapter) overHist.msgon HEAD: itsheaderfield resolves tobrain_box_msgs/Header, which is undefined.Verification
After the change, the first loop prints nothing (every
.msg/.srv/.actionon disk is now registered — 207/207), and the parser resolvesHist.headertostd_msgs/Header, matchingCeilingHistand the rest of the package. I parsed every message in the package to confirm none references a non-existentbrain_box_msgs/<type>after the edit. The change is purely additive — nothing referencedHistorbrain_box_msgs/Headerbefore — so it is backward-compatible for existing consumers; it only makes an already-authored interface available.find_package(std_msgs REQUIRED)and thestd_msgsdependency are already declared, so nopackage.xmlchange is needed. Note that PR #74 also editsCMakeLists.txt(adding unrelated new messages); this one-line addition sits next tomsg/Heartbeat.msgand should merge cleanly alongside it.How this was managed
We imported your repo's issues and pull requests into a live board and used it to manage this work. The specific story for this change is at https://eastagiletracker.com/projects/254/stories/147490, and the full board is at https://eastagiletracker.com/projects/254.
If you'd rather not receive contributions like this, reply
no-more-prson this pull request and we won't open any further ones on your repositories.Lawrence W. Sinclair
CEO / East Agile
linkedin.com/in/lwsinclair/
eastagile.com