Add Light Usercommand and include Light parameters in the componentInspector

[ahcorde]

I add a new service called /world/lights/config to be able to change the configuration of a light. This was added in the UserCommand system plugin.

As you can see in the gif the configuration can be changed in the Component Inspector.

Signed-off-by: ahcorde ahcorde@gmail.com

[chapulina]

Did a first pass 👍

Mind adding a test to test/integration/user_commands.cc?

[iche033]

what is there is no new pose? i.e. lightMsg->has_pose() is false. Should we still call SetChanged on components::Pose?

[ahcorde]

This call is able to change the light properties and the pose

[iche033]

I'm thinking of making this more efficient. Currently we are populating entityLights for all lights in the scene and going through the update process every iteration. Even though we check to see if any light properties have changed, there is still a bit of computation overhead every iteration. In an upcoming environment we are creating, we plan to put in a few hundred lights so this may add up.

How about we handle light requests similar to pose commands by adding a new components::LightCmd? When the light service request is received, we create a LightCmd component and attach it to the light entity in UserCommands, e.g. see WorldPoseCmd creation here. In RenderUtil, we will instead be looping through entities with components::LightCmd:

_ecm.Each<components::LightCmd>(
      [&](const Entity &_entity,
        const components::LightCmd *_light) -> bool
   this->entityLights[_entity] = _light->Data();
   ...

We then remove the components::LightCmd from the entity afterwards to indicate that we've processed the cmd, e.g. see removing WorldPoseCmd here

[ahcorde]

@iche033 I added most of your feedback, but I'm not sure where I should remove the LightCmd. The worldPoseCmd is remove in Physics, but where is the right place to remove these cmds? renderutils.cc?

[ahcorde]

@iche033 I removed the LightCmd, I had to convert the const-EntityComponentManager vriable in to a non-const EntityComponentManager variable to be able to remove the LightCmd. is this correct ?

https://github.com/ignitionrobotics/ign-gazebo/blob/19b3c9f06d5c097dc923324501a2a05bb889a2dd/src/rendering/RenderUtil.cc#L1162-L1169

[iche033]

hmm I see. UpdateFromECM is called in the PostUpdate function in the sensors system on the server side in the sensors system, which gets a const ref EntityComponentManager. The physics system is able to remove the component as it's done in Update function which has a mutable reference to ECM. We may have to create a new UpdateECM function in RenderUtil and call it from a new Update function (which also needs to be added) in sensors system.

On the gui side, the UpdateFromECM is called in a function that has a mutable reference to ECM. So we should be able to just do UpdateECM() and followed by UpdateFromECM

[ahcorde]

@iche033 done 0e660f9

[ahcorde]

@chapulina can you try this patch ?

--- a/src/EntityComponentManager.cc
+++ b/src/EntityComponentManager.cc
@@ -809,6 +809,9 @@ void EntityComponentManagerPrivate::SetRemovedComponentsMsgs(Entity &_entity,
   // Find the entity in the message
   auto entIter = _msg.mutable_entities()->find(_entity);
 
+  if (entIter == _msg.mutable_entities()->end())
+    return;
+
   auto it = this->removedComponents.find(_entity);
   for (uint64_t i = 0; i < nEntityKeys; ++i)
   {

[ahcorde]

I added the patch in the last commit with another segfault

[iche033]

I ran into this crash in #614 as well. There is a fix in #495. We should try to get that merged.

[ahcorde]

friendly ping @iche033 and @chapulina

[chapulina]

Thanks for iterating! I can't reproduce the weird behaviour and crash anymore. Feel free to merge once the last final details have been addressed. Be sure to also sign all commits for DCO.

Thanks! 💡