Alon

Results 17 comments of Alon

> Some more things which might also help in extracting more performance - > > https://github.com/microsoft/AirSim/blob/19ac0f265e0b81031c173ded3444be052ddaab8e/Unreal/Plugins/AirSim/Source/UnrealSensors/UnrealLidarSensor.cpp#L139-L141 > > This is just getting the first component with id != -1, instead...

Hi @zimmy87, thanks for the review, I missed the camera info somehow. I'm not entirely sure why we get this warning after applying the fix, but according to ros guidelines...

Hi @jonyMarino, that's correct. I have implemented the change you suggested which is probably the most reasonable. ![1](https://user-images.githubusercontent.com/15960497/170859861-50897f64-801c-436a-b96d-29ad4c107cff.PNG) 2 things up to your decision: 1. To follow ros convection, I...

Thanks @rajat2004! Hope you well. I will address your review soon.

@rajat2004 Thank again for the review. About removing the external cameras - I think it's a good idea. It makes the API simpler and remove an extra code. Let's make...

> Did a bit more thorough review. One thing was that it's probably better to remove the `kSimModeType*` constants from AirSimSettings. Also, with the current change, what happens when the...

@rajat2004 Sorry about that, seems like something messed up during the recent commits. It's not just the recording changes. Can you please go back and try checkout [7be0a72](https://github.com/microsoft/AirSim/pull/4340/commits/7be0a724c12f1ea010871e45d99827edb850fbb7)?

> Also, had some time so did a Windows packaging to see why the CI is failing, getting the following error - > > ``` > UATHelper: Packaging (Windows (64-bit)):...

> Again, sorry for the late reply. Did some small basic runs on Ubuntu 18.04, UE 4.25, working nicely! > > > In the current state of the PR, we're...

Hi @sakurayiannie @A-Dings, I haven't tested it on ubuntu but I don't see any reason it will cause the error. Did you run the script `clean_rebuild`? There are modifications in...