-
Notifications
You must be signed in to change notification settings - Fork 4.7k
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
ROS: Use the same settings as AirSim #3536
Conversation
Hi Rajat! I like this approach. Now there wouldn't be a necessity of 2 settings files in a distributed use case. |
Thanks a lot @rajat2004! |
Looking at this now, this will cause some compatibility issues since the latest master ROS wrapper will only work with master AirSim. Hopefully this isn't a big problem |
Yes. But that is ok for me. Thanks! |
Hi @rajat2004! This PR has broken the default sensors initialization (try running hello drone without a settings.json file, for example). Do you want to fix it? We are short of time right now. |
@jonyMarino I just tested this out, deleted the |
Ok. That's rear. David and I had that error on our machines yesterday, and the only commit changing the settings that I saw was this. Now I can't reproduce it with master. I don't know if there is a bug hidden somewhere or just a coincidence. Sorry for the false alarm, and thanks for your good disposition. |
Cool, no problem at all. I remember Unity having problems with this, but that's a separate bag of issues |
Fixes: #3530
Fixes: #3464
Fixes: #2415
Others: #2365, #2009
About
This adds an API to get the settings text which is being used by AirSim. The API is then used by the ROS wrapper to fetch the settings text and uses it to instantiate its own settings.
This felt like the cleaner way to fix the problem at its root, it'll avoid requiring the same logic to search for the settings.json file, and now doesn't matter if Airsim and the ROS wrapper are running in different machines.
Any suggestions on a better way to implement this would be great!
How Has This Been Tested?
Screenshots (if appropriate):