Multi component handling #310

shakthi-prashanth-m · 2018-03-07T09:02:20Z

Handles multiple components of a system.
Makes core to deal with standalone systems like a Land Robot, Standalone MAVLink camera, etc as well.
Changes class Device to System
Exposes generic System interface pushing back all MAVLink details in a dedicated MAVLinkSystem class. Plugins work with MAVLinkSystem class. Applications are forbidden to send MAVLink messages using System interface.
Adds integration test that discovers PX4 Autopilot and Camera component of the same.
Includes corresponding changes in backend.
Addresses

Current design: (Details omitted)

Proposed design: (Details omitted)

JonasVautherin · 2018-03-07T10:06:26Z

I am wondering one thing: why do we need dronecore to handle multiple devices? Isn't it enough to create one dronecore instance for each device and keep that complexity out of the core libraries?

More specifically, is it limiting for the user if, instead of doing something like:

DroneCore dc = new DroneCore();
dc.takeoff(UUID1);
dc.takeoff(UUID2);

the user has to do something more like:

DroneCore dc1 = new DroneCore(UUID1);
DroneCore dc2 = new DroneCore(UUID2);
dc1.takeoff();
dc2.takeoff();

shakthi-prashanth-m · 2018-03-07T10:14:12Z

When you create multiple DroneCore instance, you will get to a situation in binding same port number twice. If vehicle (autopilot & other components) are sending heartbeat to 14540, then first DroneCore instance will bind onto it, not allowing other one to handle other component.

Also by design, DroneCore Device abstracts any MAVLink component, autopilot is a special case. We can very well handle just a standalone camera with a server support using current infrastructure.

shakthi-prashanth-m · 2018-03-07T11:56:58Z

@JonasVautherin

I am wondering one thing: why do we need dronecore to handle multiple devices?

Its not multiple devices, but components. You may check #304 for issue details.

JonasVautherin · 2018-03-07T12:35:18Z

Yeah sorry, bad wording on my side :-).

I'll have a look at the issue ASAP.

julianoes · 2018-03-07T21:01:34Z

@shakthi-prashanth-m I think this API is too complex. I think all we want to connect to is a mavlink device and that device then happens to be e.g.:

an autopilot
an autopilot with camera connected
an autopilot with gimbal and camera
an autopilot with onboard computer
an autopilot with multiple cameras connected
a camera alone
a camera with gimbal

If you now want to create an interface for all these combinations it will get messy. That's why I vote to just have a device and behind that device can be whatever is supported by plugins, etc.

This is for the API, now for the internals, I think you're right in that we need to properly take care of the different components.

hamishwillee · 2018-03-07T23:36:00Z

Perhaps rambling but ...

This is a case where our old plugin model made more sense from the user perspective. In that case you would have a Device (lets call it a MAVLink "system") made up of components. DroneCore could then build up a picture of the system based on the SysIDs and component IDs and you would query the system for list of available cameras, autopilot, servos etc.

In our current design though the plugins are created "manually" as needed. That means that the coder will have to know about all the camera configurations on all the vehicles in order to build up the picture of the vehicle(s).

I "hate" it, but I think the simplest API for both implementation and users is probably to treat every component as a separate Device (referenced by sysid and component id) and we lose the idea of complete systems. You can attach any plugin to any device, but of course a camera plugin will only work when attached to a Camera Device (will ignore the mavlink messages it doesn't understand), and similarly if you add an action plugin to a Camera device it will ignore those messages.

Not sure if this works at all when you get to missions - where the system needs to be able to send camera messages.

@julianoes is this what you meant in #310 (comment) by "I vote to just have a device and behind that device can be whatever is supported by plugins, etc."

shakthi-prashanth-m · 2018-03-08T03:24:37Z

How about this ?
Let's make a hash key out of uuid/sysid and component id combination and store one device per entry in the map. No vector.
When a new component of the same system is discovered then we simply generate a hash key for that and store it.
We can return this hash key as a handle in discovery callback.
So each device is identified by hash key which we can call DroneCore Device Identifier(DDID).
Device could be any MAVLink device . Sounds good ?
So we provide methods in Device class to know what kind of device it is.
Let's keep device abstraction itself. But let's change UUID to DDID.

ddids = dc.get_device_ids();
Device &device = dc.device(ddids[0]);
if (device.is_autopilot())
....
if (device.is_camera())
....
if (device.is_gimbal())
....

julianoes · 2018-03-08T08:08:12Z

I "hate" it, but I think the simplest API for both implementation and users is probably to treat every component as a separate Device (referenced by sysid and component id) and we lose the idea of complete systems. You can attach any plugin to any device, but of course a camera plugin will only work when attached to a Camera Device (will ignore the mavlink messages it doesn't understand), and similarly if you add an action plugin to a Camera device it will ignore those messages.

At some point you might throw everything about DroneCore out and go back to mavlink 😄 .

@shakthi-prashanth-m I think a device should contain both autopilot and camera as laid out before. Therefore, you could have:

DroneCore dc;
dc.add_udp_connection(bla);
dc.add_udp_connection(foo);
Device &device = dc.device(/* or uuid for specific device */);
if (device.has_camera()) {
    // do camera things
}

hamishwillee · 2018-03-08T09:29:40Z

Yeah, lets go back to MAVLink :-)

OK, so in

DroneCore dc;
dc.add_udp_connection(bla);
dc.add_udp_connection(foo);
Device &device = dc.device(/* or uuid for specific device */);
if (device.has_camera()) {
    // do camera things
}

Is your idea than the device itself creates camera objects using a camera plugin? Or does it just know that something is a camera, hand over a component id, and you create the plugin with that?

julianoes · 2018-03-08T10:01:15Z

Is your idea than the device itself creates camera objects using a camera plugin? Or does it just know that something is a camera, hand over a component id, and you create the plugin with that?

A camera plugin goes to the core and says hey let me talk to component ID of the camera.

shakthi-prashanth-m · 2018-03-08T11:13:32Z

@julianoes we don't maintain components in device! We should ask to DroneCore class isn't it?

julianoes · 2018-03-08T14:35:09Z

we don't maintain components in device!

We should though and that's where I agree with you.

But that doesn't mean we should make the user API more complex.

shakthi-prashanth-m · 2018-03-09T00:29:45Z

Can we safely assume System ID what we get in heartbeat is unique to identify a system ?
I know that UID is unique for autopilot hardware (if provided).
What about a stand alone MAVLink camera? CSD gets system ID from autopilot heartbeat.
How does a system choose it's system ID?

If we make System ID unique, the combination of System ID and component ID uniquely identifies a MAVLink device.
Makes sense?

hamishwillee · 2018-03-09T00:57:47Z

How does a system choose it's system ID?

Any system can use whatever id it likes, provided there are no conflicts (other devices on the system that use the same ID). GCS tend to use 255 (true I believe for QGC, Mission Planner, Droid Planner). The default for PX4 is "1" (set in MAV_SYS_ID). DroneCore must have some standard id set too.

So in normal case if you were setting up a system of GCS and vehicle there would be no conflict. If you had multiple vehicles you'd set them up safely by giving consecutive numbers up from 1.

Can we safely assume System ID what we get in heartbeat is unique to identify a system ?

In a properly set up MAVLink network that would be correct. However it is quite possible for people to change the system ID, or for there to be accidental clashes.

So far we haven't assumed this - because we have a UUID in most cases that we know we can trust - we only use SYSID if we don't have UUID ... and then we trust it.

What about a stand alone MAVLink camera? I see in CSD, it fills system ID as 1. (Not sure need to check with Lalit)

For CSD this is configurable in the mavlink section of the config file. The default is actually 42, but must be over-ridden in each instance to match the associated system.

If we make System ID unique, the combination of System ID and component ID uniquely identifies a MAVLink device.
Makes sense?

Yes. Though the UUID is more unique :-), so combo of UUID and component ID perhaps :-)

shakthi-prashanth-m · 2018-03-09T04:05:26Z

Thanks @hamishwillee.
@julianoes: If it's reasonably safe to use system ID, then we better use combo of system ID and comp ID to identify a MAVLink device.
Because UUID

is that of autopilot hardware, can't be used for standalone MAVLink devices.
comes in autopilot version MAVLink message, which we don't request if device is not autopilot.

So, how about having

Device &DroneCore::device(devid_t dev_id);

to get ALL MAVLink devices. dev_id is a unique device id (sysid, compid combo).
dev_id is notified to application instead of UUID.

We could maintain std::map<devid_t, Device *> one entry per device.
If an autopilot comes with a camera there'll be two entries.
With that we can provide support to all MAVLink devices.

shakthi-prashanth-m · 2018-03-09T04:24:01Z

We can still keep uuid discovery as well. And we can also query autopilot using uuid.

Device &DroneCore::device(uint64t uuid);

To get access to autopilot of given UUID.

hamishwillee · 2018-03-09T04:59:23Z

@lbegani
Actually I do see a problem. The component ID is specified in the mavlink section here. But note, there is only one id field.

How can a single system then declare multiple separate camera components?
There are 6 component IDs for cameras. What will CSD do for the component ID if a 7th camera is detected?

EDIT:

OK, so FYI all, the docs are wrong.

Component id is not configurable (only system id)
Component ids are auto-allocated.
- Currently this is from 101 (100 is reserved for new PX4/SITL). I will raise issue to ensure that 100 is only reserved for a gazebo config so we can support all 6 camera IDs
- If more than 5 cameras are added then the system will run out of component ids. It will log an error and the new cameras will not be addressable.

Upshot, please ignore this :-)

lbegani · 2018-03-09T05:28:40Z

How can a single system then declare multiple separate camera components?

component-id field is not used. Component ID is assigned dynamically to the cameras as they get detected. Perhaps need to clean that field or add a note

There are 6 component IDs for cameras. What will CSD do for the component ID if a 7th camera is detected?

CSD will print error message and ignore additional cameras once hit the limit. It will continue running with the components already instantiated

julianoes · 2018-03-09T08:22:36Z

Can we safely assume System ID what we get in heartbeat is unique to identify a system ?

It's a mavlink limitation and therefore we need to assume it. It is up to the integrator of the whole system to make sure the system IDs don't clash and match up within a "system".

shakthi-prashanth-m · 2018-03-09T10:53:50Z

@julianoes then it's good news.

We've deprecated this and using `ActionResult` type.

cmake document recommends (below note) to avoid GLOB_RECURSE: "Note: We do not recommend using GLOB to collect a list of source files from your source tree. If no CMakeLists.txt file changes when a source is added or removed then the generated build system cannot know when to ask CMake to regenerate." Ref: https://cmake.org/cmake/help/v3.9/command/file.html

julianoes

Looks good but I had a couple of small things.

julianoes · 2018-04-03T08:32:04Z

core/mavlink_system.cpp

+    return !is_autopilot();
+}
+
+bool MAVLinkSystem::is_autopilot() const


I would change this to has_autopilot() to make it consistent and because it's not clear if something is an autopilot if it also includes a camera.

(FYI: I am unable to use Slack temporarily, but I see your email notifications.)

OK. I added is_autopilot thinking that we could say,

... if (system.is_autopilot() && system.has_camera()) print "We've a drone that has a camera" else if (system.is_autopilot()) print "We've a drone" ...

I understand your point as well, so I am ok to change as you suggested.
I am also thinking to add is_camera() for standalone cameras.
If something is a camera, it can't be anything else; because, it is drones that contain camera and other components. What do you think ?

I don't think we need is_camera() because it will be confusing to have both. And by just having has_...() it is clear that a system can always include various components.

julianoes · 2018-04-03T08:35:01Z

core/mavlink_system.cpp

+    uint8_t camera_comp_id = MAV_COMP_ID_CAMERA + (camera_id - 1);
+
+    for (auto compid : _components) {
+        return compid == camera_comp_id;


I think this is wrong and should be:

for (auto compid : _components) { if (compid == camera_comp_id) { return true; } }

No its not. The result of the expression compid == camera_comp_id evaluates to true if they're equal and false otherwise.

Oh! ok sorry I missed the point. I'll correct.

julianoes · 2018-04-03T08:37:59Z

core/mavlink_system.cpp

+std::vector<uint8_t> MAVLinkSystem::get_camera_ids() const
+{
+    std::vector<uint8_t> camera_ids;
+    camera_ids.clear();


You can just initialize it wit:

std::vector<uint8_t> camera_ids {};

julianoes · 2018-04-03T08:38:38Z

core/mavlink_system.cpp

+{
+    for (auto compid : _components)
+        if (compid == MAV_COMP_ID_GIMBAL) {
+            return compid;


Same error here, I think.

Here, I return gimbal compid if I find it.

julianoes · 2018-04-03T08:39:24Z

core/mavlink_system.h

-class Device
+
+// This represents DroneCore client application which is a GCS.
+struct GCSClient {


Please add in the comment or name what a GCS is.

julianoes · 2018-04-03T08:40:13Z

core/system.h

+    bool has_camera(uint8_t camera_id = 1) const;
+
+    /**
+     * @brief Checks whether the system has gimbal.


has a gimbal

julianoes · 2018-04-03T08:40:23Z

core/system.h

+    bool is_standalone() const;
+
+    /**
+     * @brief Checks whether the system has camera with the given camera ID.


has a camera.

julianoes · 2018-04-03T08:40:35Z

core/system.h

+    ~System();
+
+    /**
+     * @brief Checks whether the system is an autopilot.


has an autopilot.

julianoes · 2018-04-03T08:41:04Z

core/system.h

+     */
+    bool has_gimbal() const;
+
+    friend class DroneCoreImpl;


Why are the friends needed? Please add a comment explaining why.

Oh, I had not seen that. I'm interesting in why we need friends at all. I thought they were made for operators, but that they were not good practice in other uses...

Yeah good point.

friend class PluginImplBase:
While we create plugins, we pass a reference to System to plugin constructors. But eventually, plugins need access to MAVLinkSystem instance. So, we keep MAVLinkSystem as a private member of class System.
By making PluginImplBase class a friend of class System, we allow to PluginImplBase class to request access to instance of MAVLinkSystem class.

friend class DroneCoreImpl:
class DroneCoreImpl wants to perform some operations on class System, but we don't want those to expose to public. Please suggest for alternative if any.

Usually we use used the pimpl scheme to solve exactly this. I guess we can leave it for now but might want to optimize this later.

julianoes · 2018-04-03T08:46:58Z

core/plugin_impl_base.h

@@ -56,7 +59,7 @@ class PluginImplBase
    const PluginImplBase &operator=(const PluginImplBase &) = delete;

 protected:
-    Device &_parent;
+    std::shared_ptr<MAVLinkSystem> _parent;


Why didn't you leave this a reference?

class System creates a shared_ptr to MAVLinkSystem during construction to finally share them to various plugins. I made it so, because each plugin holds a shared instance of the system. It made sense to me. Do you see any issue ?

I think a reference was fine, and generally you wanted to favor references against pointers, right?

Yeah, I favor reference over a normal (raw) pointers. I can change to references as well.

I guess the question of smart pointer vs reference is about ownership: say I have an object A that has an object o. If o only lives when A lives, then you can pass a reference to other objects using o. You want to have a pointer when ownership can change, i.e. when o may live longer than A. And there you have the question between unique_ptr and shared_ptr.

I am not completely sure of the design here, but it seems like a reference is what you want, indeed.

I create MAVLinkSystem instance in System constructor. I've to use pointer here. So, as it is going to be shared by all the plugins, I made it as shared_ptr. Now, while giving access to plugins, it is better to give them as shared_ptr itself, rather than dereference and give reference. What do you think ?

@shakthi-prashanth-m why do you have to create MAVLinkSystem on the heap in the system constructor?

So that System can hide MAVLinkSystem class. Now we have pointer to it.

shakthi-prashanth-m · 2018-04-03T11:53:43Z

integration_tests/multi_components.cpp

+
+        auto is_standalone = system.is_standalone();
+
+        auto has_camera = system.has_camera(); // by default checks for camera 1


I want to ask few suggestions. I'd done testing of this by attaching a V4L2 camera on Ubuntu.
But, when I tested it today with a new camera, its component ID turned out to be 101 (usually it was 100).
The test queries whether we discovered camera 1 ? This time we discovered camera 2. As the test didn't discover camera 1, it prints as Its an Autopilot alone.

It should rather query did we discover any camera ? If yes, continue the rest of the test.

We need a method that returns true if the system has at least one camera. We have two options:

Add a new method has_any_camera().

Overload has_camera(), when 0 is passed to know whether system has at least a camera.
What shall we do ?

Interesting question, but I would not change it in this PR. What about opening an issue to discuss this point and make a new PR later with a fix for it? :-)

Later is fine. I think has_camera without argument should be true if it has any camera.

@JonasVautherin I agree.
@julianoes That would break our original idea. We thought that has_camera(), by default should check for camera 1 (to avoid passing 1 every time). I think, has_camera(0) should be OK because, by concept: Comp ID 0 indicates all IDs. So it means check for any camera on the system.

@julianoes I think what you suggested makes sense.

1. Corrected and improved `MAVLinkSystem::has_camera()` logic. 2. Use default initializer for `camera_ids` vector. 3. Add description for GCS. 4. Replace `is_autopilot()` to `has_autopilot()`. 5. Correct API descriptions of `has_camera()`, `has_gimbal()`, `has_autopilot()`. 6. Move friend decl to private of class `System` and describe them. 7. Necessary modifications in `SitlTest.MultiComponentDiscovery` test.

shakthi-prashanth-m · 2018-04-04T07:17:27Z

@julianoes @JonasVautherin
I hope f153beb addressed most of the review comments. If this is good to go now, lets merge soon after you have a re-look. We can work on the items later, that require optimization. Sounds good ?

JonasVautherin · 2018-04-04T07:23:01Z

plugins/telemetry/telemetry.cpp

    PluginBase(),
-    _impl(new TelemetryImpl(device))
+    _impl { new TelemetryImpl(system) }


I am not sure of this syntax. Why {} and not ()? I tried to change it in a rebase, but it just came back :D.

Its C++11 style to initialize class members: http://en.cppreference.com/w/cpp/language/initializer_list. See (2) in the item list.

Is it necessary, or is it just a different style? If it's only the style, I'd rather keep it consistent (we usually use () and not {} in the code). Having it different here suggests that it requires a special treatment, making the code more difficult to read (IMO).

Actually, {} is the consistent style so far used in all the plugins and core.

plugins/gimbal/gimbal.cpp: _impl { new GimbalImpl(system) } plugins/offboard/offboard.cpp: _impl { new OffboardImpl(system) } plugins/info/info.cpp: _impl { new InfoImpl(system) } plugins/follow_me/follow_me.cpp: _impl { new FollowMeImpl(system) } plugins/mission/mission_item.cpp: _impl { new MissionItemImpl() } plugins/mission/mission.cpp: _impl { new MissionImpl(system) } plugins/logging/logging.cpp: _impl { new LoggingImpl(system) } plugins/action/action.cpp: _impl { new ActionImpl(system) } plugins/telemetry/telemetry.cpp: _impl { new TelemetryImpl(system) } core/dronecore.cpp: _impl { new DroneCoreImpl() }

And it makes sense to follow C++11 style over the old as we've mentioned in README.md as well :)

Oh, my bad :)

JonasVautherin · 2018-04-04T07:23:41Z

backend/src/backend.cpp

-        Telemetry telemetry(_dc.device());
-        TelemetryServiceImpl<> telemetryService(telemetry);
+        Telemetry telemetry(_dc.system());
+        TelemetryServiceImpl<dronecore::Telemetry> telemetryService(telemetry);


TelemetryServiceImpl<> will take the default template. Again, it came back this way after your last rebase, I guess :-).

Yeah, sorry I'll correct that.

…re/DroneCore into multi-component-handling

julianoes · 2018-04-04T08:42:36Z

core/mavlink_system.cpp

 bool MAVLinkSystem::has_camera(uint8_t camera_id) const
 {
-    uint8_t camera_comp_id = MAV_COMP_ID_CAMERA + (camera_id - 1);
+    uint8_t camera_comp_id = (camera_id == 0) ?
+                             camera_id : (MAV_COMP_ID_CAMERA + (camera_id - 1));


Does that mean that the first camera has index 1? If yes, that works but should be documented that it is 1 based.

Otherwise, I'd make the default int camera_id = -1 and check for that, and then use 0 input as the first component.

Yes, although it is documented, I think what you're saying is right. Users may feel index 0 as more intuitive.

1. Make `camera_id` to start from 0. 2. Minor changes to multi-comp integration test.

julianoes · 2018-04-04T13:50:51Z

Awesome, thanks @shakthi-prashanth-m!

1. Corrected and improved `MAVLinkSystem::has_camera()` logic. 2. Use default initializer for `camera_ids` vector. 3. Add description for GCS. 4. Replace `is_autopilot()` to `has_autopilot()`. 5. Correct API descriptions of `has_camera()`, `has_gimbal()`, `has_autopilot()`. 6. Move friend decl to private of class `System` and describe them. 7. Necessary modifications in `SitlTest.MultiComponentDiscovery` test.

1. Make `camera_id` to start from 0. 2. Minor changes to multi-comp integration test.

Multi component handling

shakthi-prashanth-m requested review from mrpollo, julianoes, JonasVautherin and hamishwillee March 7, 2018 09:02

shakthi-prashanth-m added in progress enhancement core labels Mar 7, 2018

julianoes mentioned this pull request Mar 8, 2018

DroneCore handles only one component #304

Closed

shakthi-prashanth-m changed the title ~~Multi component handling~~ [WIP] Multi component handling Mar 9, 2018

shakthi-prashanth-m force-pushed the multi-component-handling branch from e3a96bd to 6b0707c Compare March 12, 2018 12:50

Shakthi Prashanth M and others added 6 commits April 3, 2018 09:40

integration_tests: Refactor MultiComponentDiscovery test

762914b

backend: Integrate Device->System changes

5552be6

core: Fix Windows build errors: C2059 and C3805

ee0bf24

plugins: Remove redandant Action::Result

1607007

We've deprecated this and using `ActionResult` type.

build_system: remove unused variable

51fbcf3

JonasVautherin force-pushed the multi-component-handling branch from 0bf2acf to 51fbcf3 Compare April 3, 2018 07:46

julianoes requested changes Apr 3, 2018

View reviewed changes

shakthi-prashanth-m commented Apr 3, 2018

View reviewed changes

Shakthi Prashanth M added 2 commits April 4, 2018 11:54

Merge branch 'develop' into multi-component-handling

d7f733f

JonasVautherin reviewed Apr 4, 2018

View reviewed changes

Merge branch 'multi-component-handling' of https://github.com/droneco…

c2ea6fa

…re/DroneCore into multi-component-handling

shakthi-prashanth-m force-pushed the multi-component-handling branch from c147002 to c2ea6fa Compare April 4, 2018 07:59

julianoes reviewed Apr 4, 2018

View reviewed changes

core, integration_tests: Address review commets #310

71f50cd

1. Make `camera_id` to start from 0. 2. Minor changes to multi-comp integration test.

julianoes approved these changes Apr 4, 2018

View reviewed changes

JonasVautherin merged commit 569d621 into develop Apr 4, 2018

JonasVautherin deleted the multi-component-handling branch April 4, 2018 14:22

shakthi-prashanth-m mentioned this pull request Apr 5, 2018

DroneCore application (GCS) should NOT mix up with System/Device #317

Closed

hamishwillee mentioned this pull request Apr 12, 2018

Change all instances and examples of Device to System mavlink/MAVSDK-docs#109

Merged

mrpollo mentioned this pull request Jun 5, 2018

v1.0.0 Changelog scratchpad #417

Closed

rt-2pm2 pushed a commit to rt-2pm2/DronecodeSDK that referenced this pull request Nov 27, 2018

core, integration_tests: Address review commets mavlink#310

de4ea6b

1. Make `camera_id` to start from 0. 2. Minor changes to multi-comp integration test.

rt-2pm2 pushed a commit to rt-2pm2/DronecodeSDK that referenced this pull request Nov 27, 2018

Merge pull request mavlink#310 from dronecore/multi-component-handling

074739d

Multi component handling

julianoes added feature and removed feature request labels Apr 23, 2020


		auto is_standalone = system.is_standalone();

		auto has_camera = system.has_camera(); // by default checks for camera 1

Multi component handling #310

Multi component handling #310

Conversation

shakthi-prashanth-m commented Mar 7, 2018 • edited by thomasgubler Loading

JonasVautherin commented Mar 7, 2018

shakthi-prashanth-m commented Mar 7, 2018

shakthi-prashanth-m commented Mar 7, 2018

JonasVautherin commented Mar 7, 2018

julianoes commented Mar 7, 2018 • edited Loading

hamishwillee commented Mar 7, 2018 • edited Loading

shakthi-prashanth-m commented Mar 8, 2018 • edited Loading

julianoes commented Mar 8, 2018

hamishwillee commented Mar 8, 2018

julianoes commented Mar 8, 2018

shakthi-prashanth-m commented Mar 8, 2018

julianoes commented Mar 8, 2018 • edited Loading

shakthi-prashanth-m commented Mar 9, 2018 • edited Loading

hamishwillee commented Mar 9, 2018

shakthi-prashanth-m commented Mar 9, 2018

shakthi-prashanth-m commented Mar 9, 2018

hamishwillee commented Mar 9, 2018 • edited Loading

lbegani commented Mar 9, 2018 • edited Loading

julianoes commented Mar 9, 2018

shakthi-prashanth-m commented Mar 9, 2018

julianoes left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

shakthi-prashanth-m Apr 3, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

shakthi-prashanth-m commented Apr 4, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

julianoes commented Apr 4, 2018

shakthi-prashanth-m commented Mar 7, 2018 •

edited by thomasgubler

Loading

julianoes commented Mar 7, 2018 •

edited

Loading

hamishwillee commented Mar 7, 2018 •

edited

Loading

shakthi-prashanth-m commented Mar 8, 2018 •

edited

Loading

julianoes commented Mar 8, 2018 •

edited

Loading

shakthi-prashanth-m commented Mar 9, 2018 •

edited

Loading

hamishwillee commented Mar 9, 2018 •

edited

Loading

lbegani commented Mar 9, 2018 •

edited

Loading

shakthi-prashanth-m Apr 3, 2018 •

edited

Loading