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

[Unity] Fix Car API #2937

Merged
merged 3 commits into from
May 24, 2021
Merged

[Unity] Fix Car API #2937

merged 3 commits into from
May 24, 2021

Conversation

rajat2004
Copy link
Contributor

@rajat2004 rajat2004 commented Aug 16, 2020

Set API enabled so that it uses API controls rather than keyboard
Plus fix some warnings

Will be easier to review commit wise

Closes #2679, closes #2636, closes #2631

Some more cleanup needs to be done, keyboard_controls_ is actually incorrect and doesn't contain the keypresses, that is actually been done here. It can be seen that the external controls are applied only when isApiEnabled is true, which is being added in this PR.
CarPawn is supposed to provide the keyboard_controls but is just a placeholder, and doesn't do anything, so I think that can be removed

TODO:

  • A method should probably be added so that CarPawnSimApi can fetch the keyboard controls from Car.cs, without this, the getCarControls API won't work to fetch the keyboard values.
  • Figure out why reverse throttle isn't working (Works in Unreal)
airsim_unity_car_api.mp4

These changes will diverge Unity implementation from Unreal one, but shouldn't be much of a problem

@rajat2004
Copy link
Contributor Author

rajat2004 commented Aug 17, 2020

@madratman @saihv @jonyMarino If possible, could you have a look at why Travis is not triggering and the Azure Pipelines status aren't getting updated? Seems to be happening for all PRs in the last 2-3 days
Thanks

Edit: Travis does get triggered, but not appearing in the CI checks - https://travis-ci.org/github/microsoft/AirSim/pull_requests

@HackBrettHB
Copy link
Contributor

Thank you, this PR solved my problem! Why is it not merged into master yet?

Copy link
Contributor

@zimmy87 zimmy87 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just some minor style changes. Tested with PythonClient\car\hello_car.py, PythonClient\car\drive_straight.py, and PythonClient\car\car_monitor.py, and the PR appears to work as intested

@rajat2004 rajat2004 force-pushed the fix-unity-car-api branch from 456e71c to 99089bf Compare May 23, 2021 15:18
@zimmy87
Copy link
Contributor

zimmy87 commented May 24, 2021

Thanks for the contribution @rajat2004!

@zimmy87 zimmy87 merged commit 6d9535a into microsoft:master May 24, 2021
@rajat2004 rajat2004 deleted the fix-unity-car-api branch May 25, 2021 01:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
4 participants