-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
Port nav2 systems test to new gazebo #4455
Comments
@SteveMacenski, so I am not going crazy, does the wait, backup, drive on heading behavior tests fail once in a while. It does not seem to be stable. |
You're not crazy, they're poorly written and it drives me bananas. I want to rewrite them at some point (#4349) to not try to fake everything out in some crazy way. I'm not really sure why this is the way that it is, I suspect it may have been written before we had any GZ simulations setup in the systems tests to work from. It is my plan to have a new contributor rewrite them as a project, but certainly if anyone else was interested in just deleting them and restarting now with the new GZ and the Simple Commander API, I think it could be done much, much more simply. But, they should generally speaking work - it shouldn't be like 50%, it would be like ~80% successful, except for the Spin test that I disabled because it was failing ~80% of the time. Wait basically never failed. Backup/Drive on Heading was like 9/10 successful. I see you have a couple in draft (#4471 #4472), is that why? |
Yes thats why they are on draft. I couldn't figure out why they fail. |
Working on the costmap filters right now and the file cleanup |
I completely rewrite the backup/spin/drive on heading tests to remove the flakiness and work with new GZ from your draft PRs. See #4515 |
@SteveMacenski Working on the GPS tests, I have migrated it, I am just having an issue with robot localization, it crashes. I am trying to figure out why. It happens after we call the fromLL service with a out of range value
|
I imagine that means that the GZ generated GPS is outside of what RL is expecting for the UTM zone. Did the other test set something for this like a datum to RL or the SDF for where to say the test is taking place w.r.t. GPS? |
@stevedanomodolor any update on the GPS test :-) The last I heard you were thinking about that but not sure where that landed |
Lots of holidays, Just came back not long ago. I will continue on this. |
No, the goal we send in the test is outside of the UTM zone. The test that fail is this one set waypoint outside of map
|
I suspect this might actually be the fix for it |
No, not recently. If you clone and use |
I am trying it now. I will let you know if it solves it as soon i get a result. |
The current solution resolves the original issue, but it introduces a new problem, which I'm unsure is the intended behavior. When the "out of range" error occurs, the system returns the default values from the fromll ROS2 service (used for coordinate conversion). In this case, those default values are zero, causing the robot to navigate to coordinates (0,0). The main issue is that the fromll service doesn't differentiate between a successful conversion and an "out of range" scenario. The root cause of the "out of range" error stems from the fact that when the fromll service is called, it assumes that the UTM zone is the same as the one initially set when the robot started the test. Since this UTM zone is never updated throughout the test, it leads to issues later on. Now that I reflect on it, this is essentially what you had pointed out earlier.
|
To solve the problem, we need to set the datum every time we cross into a new UTM zone. I changed the test point for a point inside the utm zone but outside the map and it works. However, I find it a bit confusing that we need to handle this each time we call fromll because, in most cases, we don’t know when we’re switching UTM zones. Ideally, when calling fromll, the conversion should be independent of the UTM zone, much like how it functions within the gpsFixCallback method. I opened an issue in localization for this but I don't know how useful this would be given the fact that the repo will be deprecated soon but anyways. |
Not necessary, it is nice to have to though because if someone tries to call the fromll with a value outside the utm zone set initially by the robot, RL will crash. Setting a value inside the utm zone but outside the map solved the problem in this case. |
@SteveMacenski prs are open |
@stevedanomodolor can we simply update the test so that all the points are within the same UTM zone? At the end of the day, this test is for Nav2's use of GPS waypoint navigation, not testing R_L's edge cases |
@SteveMacenski it is indeed. Without that it would not have worked hence my previous comment. All i had to do was that, put all the points in the same utm zone. |
Bug report
Required Info:
Steps to reproduce issue
Expected behavior
Actual behavior
Additional information
Feature request
Feature description
To be ported
Implementation considerations
The text was updated successfully, but these errors were encountered: