Skip to content
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

Left and Right paths are swapped #137

Closed
icemannie opened this issue Jan 21, 2019 · 28 comments · Fixed by #139
Closed

Left and Right paths are swapped #137

icemannie opened this issue Jan 21, 2019 · 28 comments · Fixed by #139
Assignees
Labels
bug Something isn't working

Comments

@icemannie
Copy link

The attached files are from a path drawn from the left load station to the left of the cargo ship. See the images. The left and right csv files (couldn't attach) show the right traveling 32 ft and the left 28 ft although the left is on the outside of the path. Also the heading is positive although Pathfinder convention has it negative.

This and other paths don't run correctly on our test robot but equivalent Vannaka Motion Profile Generator generated paths do.
left path
right path

angle2
point1
point2

Windows laptop, V10, Java 11 Path Weaver 2019.2.1

Additional context
Add any other context about the problem here.

@SamCarlberg
Copy link
Member

This is most likely just a problem with the pathfinder library. Can you try adding an intermediate waypoint to better define the path?

@icemannie
Copy link
Author

icemannie commented Jan 21, 2019 via email

@kjrobrien
Copy link
Contributor

Would you mind uploading your project or sharing the values of the points you are using so we can recreate this on our side? Thanks!

@icemannie
Copy link
Author

TempPW.zip
Zip folder of modified original path and second added test path

@icemannie
Copy link
Author

icemannie commented Jan 21, 2019 via email

@gftabor
Copy link
Contributor

gftabor commented Jan 21, 2019

Okay I am looking at the CSVs themselves (don't have pathweaver on my work computer) and it seems the sides might be flipped (left vs right).

Looking at the Angle2 we see
left Y: 3.297195 -> 10.659278
right Y: 1.127195 -> 10.689001

left X: 0.728068 -> 20.798096
right X: 0.728195 -> 22.967892

Logically the 2 sides should start at the same X and end at the same Y, which we see. However the left starts closer to the corner (0,0) than the right side and that is not the case in the CSVs.

image

Exactly whose code or where might be causing this is unclear. @icemannie does this seem consistent with your findings?

@icemannie
Copy link
Author

icemannie commented Jan 21, 2019 via email

@gftabor
Copy link
Contributor

gftabor commented Jan 21, 2019

So in answer to your "why is it this way" question. The way we handle user units is (quite clever in my opinion) we make a pane that is the size of the field in your desired units, but drawn on top of the field image. So JavaFX is handling the conversion between drawn pixels, picture pixels and user feet for instance when drawing and devs don't have to think about it.

This means that if I ever made a waypoint print its location visually in PathWeaver it would all be in user desired units, that is the "native" units of everything you see drawn in path weaver other than the image. It saves a lot of bugs related converting back and forth between visual units and user units.

It does have the consequence of (0,0) being in the upper left corner without constantly adjusting for it. Which honestly hadn't been a negative until this moment.

I guess I never noticed because I have worked with images before and just kinda assumed that's where (0,0) was.
positive y being down (as is standard in images) vs positive y being up (as is standard everywhere else)

In lastly in terms of your point that this mixup between standards of positive vs negative y is the cause of left vs right being flipped. Ya your probably right. Needs to be investigated to see if flipping left/right back is always the correct answer(as I assume) or if changing our coordinate frame is necessary to permanently fix this issue.

@icemannie
Copy link
Author

icemannie commented Jan 22, 2019 via email

@gftabor
Copy link
Contributor

gftabor commented Jan 22, 2019

https://www.chiefdelphi.com/t/pathfinder-coordinate-system/159870/7 John

Yup, the question is how to go about fixing the issue. We want to avoid breaking every path that anyone has made using pathWeaver. So minimum impact fixes are preferred. I believe just flipping left and right paths when we save them would be the only required change. (but needs someone to actually check)
More drastic changes would change all the related path files and anyone with drawn paths now would have their paths magically move when they updated.

@icemannie
Copy link
Author

First thing is to get it working correctly. I would question whether swapping left and right is enough. The heading is incorrect also. I have attached csv's from an almost identical Moton Profile Generator Path showing a - heading. Heading is used in correcting the path of the robot so it needs to be right or the robot won't follow correctly.
The only Chief Delphi post I have noticed on Path Weaver is from a team just learning trajectories. If this isn't a real sshort term fix it would be better to let teams know.
Personally I would rather redo my trajectories and have the coordinate system standardised. We anticipate maybe 5 or 6 different one this year. Anyone doing more likely does their own path planning.
If you need any testing done, I would be happy to do that.

LLDToCS_left.zip

@kjrobrien
Copy link
Contributor

Would you mind testing the following: When following a path, swap the left and right values and multiply the heading by -1?

@icemannie
Copy link
Author

icemannie commented Jan 22, 2019 via email

@icemannie
Copy link
Author

I was able to test simple left and right turns on the robot with the left side following the right path and vice versa and with heading negated.
Everything worked correctly.
I would be happy to test out the fix once you have it available.

@kjrobrien
Copy link
Contributor

John,
Per this post: https://www.chiefdelphi.com/t/pathfinder-fast-motion-profiling-for-java-and-c-tank-and-swerve-drive/150942/23?u=kjrobrien I believe the heading should not be negated given our orientation of the coordinate system.

Positive headings are going from X+ towards Y+

This is what is happening in the path Angle2 that you uploaded, so I do believe the headings are correct given our orientation of the coordinate system. (Whether that system makes sense going forward is up for debate :) ) Now, this may require users to orient themselves to our axis in their follower code (by negating the angle, etc.).

However, the left and right paths are being swapped, unfortunately. I have attached a .zip file for a build of PathWeaver that I'd like you to try and see if the output paths are in line with what you would expect. If you have the time, we would love if you could try loading the outputs onto a robot. If this behaves as expected, my goal would be to release this in the next update, with a note informing users that paths should be regenerated.
pathweaver-2019.2.1-invert-fix.zip

.
Thanks for your help & guidance,
Kevin

@icemannie
Copy link
Author

icemannie commented Jan 23, 2019

The college where we have our robot is closed today for weather conditions. It may be open this evening.
I will build and look at the CSVs later this morning. One change that you should - even must - make to help the change get accepted is to add a grid ruler to x and y axes on the image. I simply made the assumption that y0 was bottom left and struggled with the paths not performing on the robot.

@icemannie
Copy link
Author

icemannie commented Jan 23, 2019 via email

@bradamiller
Copy link

John - thanks for all your help testing and tracking down the problem. Seems like the right thing to do is let people know that the issue exists today so they can make the 2 line change to their code to work around the problem. Then we can fix it correctly in the next update with a clear description of the issue in the release notes.

@icemannie
Copy link
Author

Brad, I agree wih that and will not test the version Kevin sent me. I posted on Chief Delphi this morning about the situation and so far at least not a single comment. That and no earlier questions seems to indicate low usage currently or teams aren't at that point yet. Perhaps the driver control in sandstorm autonomous is making teams less inclined to make the effort.
I have tested Vannaka Motion Profile Generator 4.0 and it works with standard equations.

@kjrobrien
Copy link
Contributor

This decision has been reflected in the documentation on screensteps here:
https://wpilib.screenstepslive.com/s/currentCS/m/84338/l/1021631-integrating-path-following-into-a-robot-program and has been documented on the list of known issues here: https://wpilib.screenstepslive.com/s/currentCS/m/getting_started/l/1028964-known-issues

@wrall
Copy link

wrall commented Jan 27, 2019

I'm not sure that the solution of swapping left and right paths is universally the case. I have some paths that work correctly and some that don't. My paths basically always turn toward the left, even if they're supposed to turn towards the right. A student made 5 paths (go straight, turn left, turn right, make clockwise circle, and drive a sine wave). They all turn left (or go in a counter-clockwise circle). Could there be a more fundamental problem?

@icemannie
Copy link
Author

icemannie commented Jan 27, 2019 via email

@wrall
Copy link

wrall commented Jan 27, 2019

The paths generated for the below have basically the same right and left position values, turning left. The starting position for left and right are 0 inches, and the ending position for both paths are approximately 58" for left, 96" for right.
image

I could swap left and right for one of them, but it would be useful to know when that might happen without inspecting the paths too closely.

Also, for one of the paths I generated which was supposed to drive straight forwards 48", the last position values generated are only 46.5".

@kjrobrien
Copy link
Contributor

@wrall I cannot personally reproduce what you have described. With the current version of PathWeaver, the left and right paths are flipped when I create similar paths to the two you described. Could you please share your project with me? My instinct on the drive straight issue is that it MAY be related to Pathfinder itself, you may need to have a shorter time step (i.e. generate more points).

@wrall
Copy link

wrall commented Jan 27, 2019

Smaller timestep does seem to allow it to reach the actual destination. Very strange behavior.

See paths below:
PathWeaver.zip

@kjrobrien
Copy link
Contributor

So, I admit that this is not straight-forward and we need to come up with a better way of signifying this:
image
But when I drag the path, I can see that what you "think" is the starting point, is actually the ending point. Therefore, it makes sense that the left and right do not have to be switched for this scenario, as the path as being configured is not correct. When I switch the starting and ending waypoints, I have the issue we have been finding. Calculating takes a decent amount of time. So when the paths are being dragged, we create an estimate of what the final path will be. Due to some idiosyncrasies with Pathfinder (our dependency for path generation), it created what looked like a valid path, but was actually the robot driving from near the cargo ship back to the HAB. Therefore, through my testing (I've created paths turning left and right from each quadrant of the field), I believe that the left and right sides do indeed need to be switched for all paths.

@kjrobrien kjrobrien changed the title Strange Pathweaver Paths Results Left and Right paths are swapped Jan 27, 2019
@kjrobrien kjrobrien self-assigned this Jan 27, 2019
@kjrobrien kjrobrien added the bug Something isn't working label Jan 27, 2019
@wrall
Copy link

wrall commented Jan 27, 2019

Ah, you're right - I assumed that the path was correct based on how it displayed, having not made them myself.

Some suggestions:

  • Display a little number within each waypoint triangle identifying the order, or color-code them somehow.
  • Change the Waypoint Properties pane to be in an ordered list format, with each row one showing all of those 5/6 (editable) columns.
  • Provide the ability to add a new waypoint to the end of the path instead of having to create the first and last, then specify the middle.

@kjrobrien
Copy link
Contributor

I like all of these ideas! I've opened an issue for a feature request #138.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants