-
Notifications
You must be signed in to change notification settings - Fork 14.8k
Fix issue where XRCE_DDS Bridge did not respect environment variables #25231
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix issue where XRCE_DDS Bridge did not respect environment variables #25231
Conversation
beniaminopozzan
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thanks @sansha , could you set the default value for the IP address in simulation to 127.0.0.1 to maintain back-compatibility?
Hey, not sure what you mean. I can set the default to 127.0.0.1 (see 59a751d) but the problem I had was in simulation, so I would like to also change it in simulation. Maybe outside of sim we want to keep it to 127.0.0.1? |
|
@sansha , outside the sim, when you set So, please make the default value for UXRCE_DDS_AG_IP equal to |
|
@beniaminopozzan thank you for the feedback and apologies for taking so long. I added setting a default value earlier in the script. Is this what you intended? Do you think this fix can make it into 1.16.0? Would be amazing to have this in and being able move to a stable version. |
|
CI Failing with MAVROS Mission Tests is not related I think, will try resolving by merging main. |
beniaminopozzan
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @sansha !
|
@sansha yes, it can go in 1.16! Could you open a backport pr on the |
* Read XRCE_DDS IP from ENV * just remove the -h flag from the launch command * add 127.0.0.1 as default IP * add default value for IP
|
@beniaminopozzan awesome, I opened a backport PR here: #25299 |
* Read XRCE_DDS IP from ENV * just remove the -h flag from the launch command * add 127.0.0.1 as default IP * add default value for IP
* Read XRCE_DDS IP from ENV * just remove the -h flag from the launch command * add 127.0.0.1 as default IP * add default value for IP
* Read XRCE_DDS IP from ENV * just remove the -h flag from the launch command * add 127.0.0.1 as default IP * add default value for IP
* Read XRCE_DDS IP from ENV * just remove the -h flag from the launch command * add 127.0.0.1 as default IP * add default value for IP
* Read XRCE_DDS IP from ENV * just remove the -h flag from the launch command * add 127.0.0.1 as default IP * add default value for IP
Solved Problem
Fixes #25188 (detailed description in that issue)
Solution
Hi, this is the PR fixing #25188 by removing the
-h 127.0.0.1flag in the launch command for the client.This fix works for me, not sure if other setups are affected.
Let me know your thoughts!
Changelog Entry
For release notes:
Alternatives
In the first commit I load the env variable explicitly but changed this to just remove the flag since I cannot test this right now. But this of course would also be an option
Test coverage
Context
Related links, screenshot before/after, video