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

New sensor system that aims to make easier adding new sensors #830

Merged
merged 32 commits into from
Oct 23, 2018

Conversation

nsubiron
Copy link
Collaborator

@nsubiron nsubiron commented Oct 2, 2018

Description

The idea is to create a new sensor system that allows contributors to "plug in" their implementations easily.

  1. Add a sensor actor.
  2. Add a sensor serializer (and possibly a sensor data view in Python).
  3. Register the classes.

All the details below.

This PR fixes:

TODO:

  • Test that the sensor transform received is in world coordinates.
  • Test the resulting images on Windows, and if possible with Vulkan.

Where has this been tested?

  • Platform(s): Ubuntu 16.04
  • Python version(s): 2.7
  • Unreal Engine version(s): 4.19

Possible Drawbacks

I've refactored some of the code that reads the image buffers and fixed a possible bug, this part is a bit tricky and should be tested in Windows to make sure everything works as expected (in Linux does), i.e., getting some images with different (odd) sizes in the client side to see they make sense.


Mini tutorial

This are the steps to add a new sensor (I will add a tutorial with more detail soon):

1. Sensor Actor

Create a sensor actor deriving from ASensor (or one of its derived classes), this class requires to implement the following methods:

  • static FActorDefinition GetSensorDefinition(); Returns an actor definition that will be given to the user in the client-side to configure the details of the sensor.
  • void Set(const FActorDescription &ActorDescription) override; Configure the sensor with the attributes that the user requested.
  • void Tick(float DeltaTime) override; This function is called on every game update and here the sensor is expected to produce the data and send it (more on this below).
  • Also requires the UE4 macros, UCLASS() and GENERATED_BODY().

The following sensor classes are already implemented here: ASceneCaptureCamera, ADepthCamera, ASemanticSegmentationCamera, and ARayCastLidar.

This class should be added to Unreal/CarlaUE4/Plugins/Carla/Source/Carla/Sensor folder.

ASensor contains a FDataStream for sending the data through the streaming server. Sensor actors should write their measurements to this stream using one of the two methods in the stream, Send_Async or Send_GameThread depending in which thread they're producing the data. The class FDataStream is quite well documented, take a look for more details.

The stream methods won't compile however until the sensor and its serializer are registered, which takes us to the next class required.

2. Sensor Serializer

This class is in charge of serializing and deserializing the data. The only requirement of this class is providing two static methods with the following signatures:

  • static Buffer Serialize(const YourASensor &sensor, ...data here...); This function should convert the data provided by the sensor actor into a Buffer object. The signature is quite open.
  • static SharedPtr<SensorData> Deserialize(RawData data); This function has to convert a RawData object (which is a wrapper around a buffer with some extra meta-information) into a SensorData derived class.

Serialization happens in the simulator-side, right after writing the data to the stream. Deserialization happens in the client-side, right after delivering the data to the user in the callback function provided to the listen method of the sensors.

This class should be added to LibCarla/source/carla/sensor/s11n folder, and its corresponding namespace.

There are already some SensorData classes implemented, like images and arrays, otherwise you must add a new class fitting your needs. In this case the class must also be exposed to Python. I will add more info on this in the tutorial.

3. Register your classes

Your classes must be registered within the SensorRegistry as an std::pair<Sensor *, Serializer>. Also follow the steps in the header as the includes must be added in the right places.

// LibCarla/source/carla/sensor/SensorRegistry.h

using SensorRegistry = CompositeSerializer<
  std::pair<ASceneCaptureCamera *, s11n::ImageSerializer>,
  std::pair<ADepthCamera *, s11n::ImageSerializer>,
  std::pair<ASemanticSegmentationCamera *, s11n::ImageSerializer>,
  std::pair<ARayCastLidar *, s11n::LidarSerializer>
>;

And that's it, the sensor registry now do its compile-time magic to dispatch the right data to the right serializer.

Well, ok, I might have missing something. We will be polishing this guide as we start playing around with new sensors.


This change is Reviewable

@nsubiron nsubiron added this to the 0.9.1 milestone Oct 2, 2018
@nsubiron nsubiron self-assigned this Oct 2, 2018
@ghost ghost added the review label Oct 2, 2018
@harlowja
Copy link
Contributor

Is there anyway to split this PR into smaller pieces?

@nsubiron
Copy link
Collaborator Author

...and this is nothing compared to the ones to come 😰
That'd be a good practice indeed, but we are in such crazy deadline that we are gonna have to merge most of the things in bulk. Feels like back to the times I was the only developer.

@harlowja
Copy link
Contributor

That's sad :-/

@harlowja
Copy link
Contributor

🤗

@harlowja
Copy link
Contributor

What is the crazy deadline btw?

@nsubiron
Copy link
Collaborator Author

We were requested to have a 0.9.x API complete, with all the measurements and road info, so people can start moving to new API asap. Our plan is to have a 0.9.1 fully functional (this week maybe), even if some things are still a bit hacky. Afterwards we'll make it more robust and fast, but without changing the API.

Copy link
Contributor

@marcgpuig marcgpuig left a comment

Choose a reason for hiding this comment

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

I'll need more coffee to continue reviewing in depth. Good job.

Reviewed 115 of 115 files at r1.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @nsubiron)


Unreal/CarlaUE4/Plugins/Carla/Source/Carla/Sensor/DataStream.h, line 125 at r1 (raw file):

#endif // WITH_EDITOR
  check(Stream.has_value());
  (*Stream).Write(

Just curious why you don't use ->, will do the job anyway right?

Copy link
Contributor

@marcgpuig marcgpuig left a comment

Choose a reason for hiding this comment

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

Reviewed 1 of 115 files at r1.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Plugin system for adding new sensors Add Lidar to new API
3 participants