Skip to content

[Collision Monitor] fix Z base frame transformation#5794

Merged
SteveMacenski merged 1 commit intoros-navigation:mainfrom
botsandus:fix_cm_z_transform
Dec 25, 2025
Merged

[Collision Monitor] fix Z base frame transformation#5794
SteveMacenski merged 1 commit intoros-navigation:mainfrom
botsandus:fix_cm_z_transform

Conversation

@doisyg
Copy link
Contributor

@doisyg doisyg commented Dec 17, 2025

min_height / max_height applied to a pointcloud source in the CM is not anymore relative to base_frame_id but relative to the frame_id of that source when not using use_global_height.
Can be quite critical as impacting which part of a pointcloud are used in the collision monitor/detector.
I believe since: #5586
Draft to get the info out asap but not yet 100% checked. Will need extra sanity checks.

fix Z base frame transformation
6ed1345 
Signed-off-by: Guillaume Doisy <guillaume@dexory.com>
@doisyg doisyg marked this pull request as draft December 17, 2025 18:46
@doisyg doisyg mentioned this pull request Dec 17, 2025
Merged
8 tasks
@SteveMacenski
Copy link
Member

SteveMacenski commented Dec 17, 2025

@greganderson-vermeer can you please comment / test?

@doisyg
Copy link
Contributor Author

doisyg commented Dec 17, 2025 •
edited
Loading

Tested now on our platform. Fixes our issue. But that was in order to have a fast and quick hotfix.
@greganderson-vermeer I will let you check the generalization of that fix if you can.
Need more test

@doisyg
Copy link
Contributor Author

doisyg commented Dec 18, 2025

Properly tested on Dexory robot now.

@doisyg doisyg marked this pull request as ready for review December 18, 2025 17:09
greganderson-vermeer
greganderson-vermeer reviewed Dec 23, 2025
View reviewed changes
Copy link
Contributor

@greganderson-vermeer greganderson-vermeer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This all looks good to me.

mini-1235
mini-1235 approved these changes Dec 24, 2025
View reviewed changes
Copy link
Collaborator

@mini-1235 mini-1235 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me as well

@doisyg doisyg removed the request for review from tonynajjar December 24, 2025 13:29
@doisyg
Copy link
Contributor Author

doisyg commented Dec 24, 2025

I guess merge will wait for Steve to come back
image

@SteveMacenski SteveMacenski merged commit 4534ff1 into ros-navigation:main Dec 25, 2025
16 of 17 checks passed
@SteveMacenski
Copy link
Member

SteveMacenski commented Dec 25, 2025

Still working through permissions and workflows. I think in the new year Maurice will get write access which also comes with merge abilities. I wish github gave me more granular settings - happy to have Maurce merge things himself, but write permissions I want to be more cautious about for the first few weeks

mergify bot pushed a commit that referenced this pull request Dec 25, 2025
@doisyg @mergify
fix Z base frame transformation (#5794)
9ff63cd 
Signed-off-by: Guillaume Doisy <guillaume@dexory.com>
Co-authored-by: Guillaume Doisy <guillaume@dexory.com>
(cherry picked from commit 4534ff1)
@mergify mergify bot mentioned this pull request Dec 25, 2025
Merged
SteveMacenski pushed a commit that referenced this pull request Dec 25, 2025
@mergify @doisyg
fix Z base frame transformation (#5794) (#5815)
3fc1600 
(cherry picked from commit 4534ff1)

Signed-off-by: Guillaume Doisy <guillaume@dexory.com>
Co-authored-by: Guillaume Doisy <doisyg@users.noreply.github.com>
Co-authored-by: Guillaume Doisy <guillaume@dexory.com>
mergify bot pushed a commit that referenced this pull request Dec 25, 2025
@doisyg @mergify
fix Z base frame transformation (#5794)
ede32e9 
Signed-off-by: Guillaume Doisy <guillaume@dexory.com>
Co-authored-by: Guillaume Doisy <guillaume@dexory.com>
(cherry picked from commit 4534ff1)
@mergify mergify bot mentioned this pull request Dec 25, 2025
Merged
SteveMacenski pushed a commit that referenced this pull request Dec 25, 2025
@mergify @doisyg
fix Z base frame transformation (#5794) (#5816)
7482c5d 
(cherry picked from commit 4534ff1)

Signed-off-by: Guillaume Doisy <guillaume@dexory.com>
Co-authored-by: Guillaume Doisy <doisyg@users.noreply.github.com>
Co-authored-by: Guillaume Doisy <guillaume@dexory.com>
pele1410 pushed a commit to Metal-Shark-Sharktech/navigation2 that referenced this pull request Dec 30, 2025
@doisyg
fix Z base frame transformation (ros-navigation#5794)
de3b1de 
Signed-off-by: Guillaume Doisy <guillaume@dexory.com>
Co-authored-by: Guillaume Doisy <guillaume@dexory.com>
Signed-off-by: Christopher Thompson <cthompson@metalsharkboats.com>
Lotusymt pushed a commit to Lotusymt/navigation2 that referenced this pull request Jan 16, 2026
@doisyg @Lotusymt
fix Z base frame transformation (ros-navigation#5794)
c192139 
Signed-off-by: Guillaume Doisy <guillaume@dexory.com>
Co-authored-by: Guillaume Doisy <guillaume@dexory.com>
Pana1v pushed a commit to Arnav-panjla/navigation2 that referenced this pull request Feb 21, 2026
@doisyg
fix Z base frame transformation (ros-navigation#5794)
840c311 
Signed-off-by: Guillaume Doisy <guillaume@dexory.com>
Co-authored-by: Guillaume Doisy <guillaume@dexory.com>
redvinaa pushed a commit to EnjoyRobotics/navigation2 that referenced this pull request Mar 2, 2026
@mergify @doisyg @redvinaa
fix Z base frame transformation (ros-navigation#5794) (ros-navigation…
8097630 
…#5815)

(cherry picked from commit 4534ff1)

Signed-off-by: Guillaume Doisy <guillaume@dexory.com>
Co-authored-by: Guillaume Doisy <doisyg@users.noreply.github.com>
Co-authored-by: Guillaume Doisy <guillaume@dexory.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@mini-1235 mini-1235 mini-1235 approved these changes

+1 more reviewer

@greganderson-vermeer greganderson-vermeer greganderson-vermeer left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

backport-jazzy backport-kilted

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@doisyg @SteveMacenski @mini-1235 @greganderson-vermeer