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

Combine Lidar Segmentation API into getLidarData #2810

Merged
merged 4 commits into from
Mar 1, 2021

Conversation

rajat2004
Copy link
Contributor

@rajat2004 rajat2004 commented Jun 29, 2020

Having separate APIs means that if we want the segmentation data corresponding to the point cloud, it has to be called just after getLidarData
Even this is not foolproof, and might get mismatched data. Related issue #2409
Combining the 2 APIs seemed to be to a cleaner solution. Also add generic findSensorByName function in VehicleApiBase

A simple test-script -

import setup_path
import airsim
import time

client = airsim.CarClient()
client.confirmConnection()

count = 0

for i in range(1000):
    lidarData = client.getLidarData()
    # lidarSegData = client.simGetLidarSegmentation()
    lidarSegData = lidarData.segmentation
    
    if not lidarData.point_cloud:
        print("Null entry, skipping")
        continue

    print(f"Lidar: {len(lidarData.point_cloud)}")
    # time.sleep(0.1)
    print(f"LidarSeg: {len(lidarSegData)}")

    if(len(lidarSegData) != len(lidarData.point_cloud)//3):
        print("Not equal")
        count+=1
    time.sleep(0.1)

print(count)

For envs like Neighbourhood, I'm getting 30-40 out of 1000 mismatched entries (which isn't a lot actually)

Note: I haven't added the workaround for empty segmentation vecotr similar to what is there for point_cloud in RpcLibAdaptorsBase since rpclib/rpclib#152 has been fixed. I've opened #2812 for removing the workaround, hopefully they rebase cleanly
With the removal of the workaround, checking whether the point_cloud is empty using if not lidarData.point_cloud: works

Edit: Well, after thinking about it a bit more, it somewhat makes sense that the segmentation API is a different one since this data won't actually be present in real life, therefore sim prefix. This problem can also be detected pretty easily, so if we want to keep the APIs separate, the first option could be to close this PR (or just use the last commit)
Another could be to create a new Lidar sensor which has the segmentation info as well. This will also lead to the removal of collecting segmentation data in the normal Lidar sensor, which probably will improve performance (but will need to check how much it's affected)
Let me know whatever seems best

@madratman
Copy link
Contributor

/azp run microsoft.AirSim

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@zimmy87
Copy link
Contributor

zimmy87 commented Mar 1, 2021

tested this locally and it works for me so I'm going ahead and merging this

@zimmy87 zimmy87 merged commit 2e4881c into microsoft:master Mar 1, 2021
@rajat2004 rajat2004 deleted the lidar branch March 1, 2021 15:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants