[roadmngr] Added hintRoad param to XYZH2TrackPos#280
Open
brifsttar wants to merge 1 commit intoesmini:masterfrom
Open
[roadmngr] Added hintRoad param to XYZH2TrackPos#280brifsttar wants to merge 1 commit intoesmini:masterfrom
brifsttar wants to merge 1 commit intoesmini:masterfrom
Conversation
Collaborator
|
Thanks for this interesting proposal. I haven't had time to look into it yet, and I realize it's not a 5 minute task to digest :) |
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.
Hi Emil,
This PR is a new feature, and not a bugfix, and as so isn't really important, and I would definitely understand if it got rejected, as it has drawbacks.
The goal of this change is to solve the issue of "non-optimal" computed track position when a vehicle gets into a junction. For obvious reasons, when getting into a junction, the
XYZH2TrackPoshas a difficult job of choosing between all junctions' road connected to the incoming road. Currently, this is done by selected the "straight most" junction road (though it's not entirely true, more on that later).esmini/EnvironmentSimulator/Modules/RoadManager/RoadManager.cpp
Lines 6469 to 6477 in 75574a4
However, during simulation, for nearly all vehicles (except user-controlled ones) we have some way of knowing which road the car should follow in the junction. For example, if we told the car controller to turn left, that information could be very useful for
XYZH2TrackPoswhen trying to select a new road when entering the junction.To that end, I added a new parameter to
XYZH2TrackPoscalledhintRoad, which, if specified, will give priority for this road to be selected when encountering a junction. I didn't forward the parameter "upward" toXYZ2TrackorSetInertiaPos(which is what I use on my side), because I didn't want to "pollute" all those methods. So to use the parameter, you have to call directlyXYZH2TrackPosinstead of relying on higher level methods, which isn't that big of a deal in my opinion (I updated my code to do that).On a related note, and as mentioned above, I think there is a strange (but maybe intended) behavior on
XYZH2TrackPos's default "straight most" selection. As we can see on the code above, it relies on the road's curvature, which comes fromesmini/EnvironmentSimulator/Modules/RoadManager/RoadManager.cpp
Line 6256 in 75574a4
esmini/EnvironmentSimulator/Modules/RoadManager/RoadManager.cpp
Line 2357 in 75574a4
That code itself is valid, but from what I understand, it computes curvature at the junction road "inner" connection, i.e., towards the incoming road. This can have side effects in junctions where roads start with a straight geometry, as seen below. In such case, the computed curvature is exactly the same between roads, and the first road iterated over gets selected, which in this junction's case is the left turn.
So it might be more interesting to instead use the "outer" connection's curvature to have a better estimation of the "straight most" road. The easy way to do it would be to change the behavior of
IsDirectlyConnectedto return the "outer" curvature, but since there might be existing code elsewhere relying on the current behavior, I don't want to potentially break things.If you want, I can make a separate issue for that last point, or submit a PR with the behavior changed on
IsDirectlyConnected(as discussed above). But since the new "hint" feature also kind of supersedes the default behavior, this issue might also be deemed not important.