Add support for custom JSON constructors for vehicle sub-systems

[edvinsternvik]

Currently vehicles can be constructed from JSON files, but only using the predefined templates. This pull request adds the possibility to supply custom constructors when creating a vehicle from a JSON file.

This can be used like this:

(main.cpp)

// An example of a custom vehicle component
class ExampleDriveline : public ChDrivelineWV {
    // Implementation
};

// An example constructor for custom driveline implementations
std::shared_ptr<ChDrivelineWV> custom_driveline_constructor(
    const std::string& name,
    const rapidjson::Document&
) {
    if(name == "ExampleDriveline") {
        return std::make_shared<ExampleDriveline>();
    }
    else if(name == "ExampleDriveline2") {
        return std::make_shared<ExampleDriveline2>();
    }
    // etc...

    return nullptr;
}

int main() {
    // Set custom constructors
    CustomConstructorsWV custom_constructors = {};
    custom_constructors.driveline_constructor = custom_driveline_constructor;
    // custom_constructors.suspension_constructor = ....

    // Create vehicle
    WheeledVehicle vehicle(
        vehicle::GetDataFile("vehicle/Example.json"),
        ChContactMethod::NSC,
        false,
        true,
        custom_constructors
    );
}

(ExampleDriveline.json)

{
  "Name":       "Example Driveline",
  "Type":         "Driveline",
  "Template":  "ExampleDriveline",

  "Stuff":
  {
    "Thing": 0.5
  }
}

[m4rr5]

First of all, thanks for contributing this PR, @edvinsternvik. I've made a similar change to my local version of Chrono that allows me to do pretty much the same thing you are proposing. I have a few questions about your implementation:

1. Do I correctly assume that in the example code you provide, you would actually pass a reference to the parsed JSON document as an argument of the ExampleDriveline constructor?
2. When looking at the if (name == "A") {} else if (name == "B") {} code, my first thought is, why don't you create a map where the template name is the key and the "constructor" (or builder function) is the value? Do you have a use case where you would construct templates in a different way that would not work with a map?
3. Passing (parts of) the CustomConstructorsWV structure around as an argument seems to cause quite a few changes in the codebase. Would it make sense to consider some kind of "registry" that you can use to register your constructors that gets queried by the different functions that create objects based on templates?

Looking forward to your thoughts as well as feedback from others in the project.

[edvinsternvik]

First of all, thanks for contributing this PR, @edvinsternvik. I've made a similar change to my local version of Chrono that allows me to do pretty much the same thing you are proposing. I have a few questions about your implementation:

1. Do I correctly assume that in the example code you provide, you would actually pass a reference to the parsed JSON document as an argument of the ExampleDriveline constructor?

2. When looking at the if (name == "A") {} else if (name == "B") {} code, my first thought is, why don't you create a map where the template name is the key and the "constructor" (or builder function) is the value? Do you have a use case where you would construct templates in a different way that would not work with a map?

3. Passing (parts of) the CustomConstructorsWV structure around as an argument seems to cause quite a few changes in the codebase. Would it make sense to consider some kind of "registry" that you can use to register your constructors that gets queried by the different functions that create objects based on templates?

Looking forward to your thoughts as well as feedback from others in the project.

Hello, thanks for your feedback.

Yes, the example should probably be something more like:

// An example constructor for custom driveline implementations
std::shared_ptr<ChDrivelineWV> custom_driveline_constructor(
    const std::string& name,
    const rapidjson::Document& d
) {
    if(name == "ExampleDriveline") {
        return std::make_shared<ExampleDriveline>(d);
    }
    else if(name == "ExampleDriveline2") {
        return std::make_shared<ExampleDriveline2>(d);
    }
    // etc...

    return nullptr;
}

Good suggestion, I can't think of a use case that wouldn't work with a map. I just decided to just follow the method that was already used in the code
(for example, the drivelines are constructed in ChUtilsJSON like so:)

    if (subtype.compare("ShaftsDriveline2WD") == 0) {
        driveline = chrono_types::make_shared<ShaftsDriveline2WD>(d);
    } else if (subtype.compare("ShaftsDriveline4WD") == 0) {
        driveline = chrono_types::make_shared<ShaftsDriveline4WD>(d);
    // ....

but using a map might be a cleaner approach.

Yes a global registry would reduce the number of changes required in the code. I decided not to do it that way to avoid unnecessary global state as I generally think they make the code harder to reason about. But in this case the constructors are only going to be used in a handful of places so a global registry might be better than having to pass the constructors around everywhere.

I welcome any opinions on point 2 and 3, so that I can modify the code based on what we decide.

[rserban]

Edvin, thank you for the contribution.
I would need to think a bit more about this, but right now I simply do not have the bandwidth. Furthermore, should we decide to merge this PR, it would need to include a little bit more (some documentation, proper examples, etc).
We are preparing for a new release soon and it is too close to that for including a change like this. I suggest we keep this open and revisit it after the next release.