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

More actor components and follow plugin #157

Merged
merged 3 commits into from
Jun 5, 2020
Merged

More actor components and follow plugin #157

merged 3 commits into from
Jun 5, 2020

Conversation

chapulina
Copy link
Contributor

@chapulina chapulina commented May 27, 2020

Addresses part of #91.

actor_follow_4

Add a couple of components:

  • AnimationName
  • AnimationTime

Also added the FollowActor plugin as a useful* example, which is a port of servicesim::FollowActorPlugin, with a subset of functionalities:

  • An entity to follow can be set on the SDF (<target>)
  • A minimum distance from the target's origin can be set
  • If the target is further than the maximum distance, the actor stops following (this was useful in the ServiceSim context, maybe not so much outside of it)
  • In case the actor has many animations, a specific animation one can be chosen in the SDF
  • The actor's velocity is set on the SDF

* Actually useful, unlike the examples/plugin/command_actor I had been adding.


⚠️ One inconvenient part is having to manually set the "animation X velocity", which is different for each animation file.

I looked into calculating it automatically inside rendering like we do when using interpolate_x. That approach involves calculating the average speed of the animation based on the start and end positions of the root node. That's only an approximation though. Plugin developers have more flexibility if they can directly set AnimationTime at each time step. That's more flexible but also more work. I took that route here mainly because that interface was exposed in Gazebo-classic's Actor::SetScriptTime, which I saw used on at least one other plugin besides ServiceSim's.

In the future, we could look into exposing the skeleton information to all systems as a component. We need to be careful when the SceneManager is only on the client though.

@chapulina chapulina requested a review from iche033 as a code owner May 27, 2020 20:57
@chapulina chapulina self-assigned this May 27, 2020
@github-actions github-actions bot added the 🔮 dome Ignition Dome label May 27, 2020
@codecov
Copy link

codecov bot commented May 27, 2020

Codecov Report

Merging #157 into master will increase coverage by 0.35%.
The diff coverage is 20.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #157      +/-   ##
==========================================
+ Coverage   65.42%   65.77%   +0.35%     
==========================================
  Files         128      128              
  Lines        6287     6297      +10     
==========================================
+ Hits         4113     4142      +29     
+ Misses       2174     2155      -19     
Impacted Files Coverage Δ
include/ignition/gazebo/components/Actor.hh 27.27% <20.00%> (-72.73%) ⬇️
src/SimulationRunner.cc 87.54% <0.00%> (+1.18%) ⬆️
src/LevelManager.cc 89.25% <0.00%> (+3.03%) ⬆️
src/SdfEntityCreator.cc 86.90% <0.00%> (+3.63%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update fdd1ce3...e9588c2. Read the comment docs.

Copy link
Member

@luca-della-vedova luca-della-vedova left a comment

Choose a reason for hiding this comment

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

Looks good! I have a question about the high level behavior, I have seen that the Z axis is usually ignored which means that the actors will only follow objects in the XY plane (i.e. attached below).
image
This would make the behavior in cases where the world is not a perfectly flat plane a bit tricky, let's say you have a robot going up a slope and a human following it, if I understand correctly the human will keep walking forward, probably going through the plane, while the robot goes up and it will end up in a configuration similar to the screenshot.

On the other hand though, I can see how if you introduced always following Z it could require some tuning in the case of flat planes, i.e. if you set the animation root node of the example to follow the root node of the cube object the actor could be partially floating / compenetrating the plane depending on the relative position of the two root nodes.

What do you think?

Comment on lines +198 to +201
// Time delta
std::chrono::duration<double> dtDuration = _info.simTime -
this->dataPtr->lastUpdate;
double dt = dtDuration.count();
Copy link
Member

Choose a reason for hiding this comment

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

More a curiosity here, can we just use _info.dt or is there any benefit in calculating the dt manually?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah good point. I was planning to throttle this system so it doesn't run at 1000 / 500 Hz. I'll go ahead and do that.

@chapulina
Copy link
Contributor Author

the actors will only follow objects in the XY plane

Yeah you're right, that's the current assumption. I can't think right now of a solution that is generic enough though. The trick is how to choose the Z for the actor. I think the ideal solution would involve detecting the closest point on the ground below the actor and keeping a fixed* distance to that.

Think for example of an actor following a drone - the drone may go up and down but the actor should keep the same Z on flat ground.

Let me know if you have a specific use case and I can try to at least support that 🤔

* Depending on the animation even a fixed distance may be wrong though.

Copy link
Contributor

@iche033 iche033 left a comment

Choose a reason for hiding this comment

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

nice, works well for me!

one behavior of the follow actor system I noticed is that if the box moves of out range and back within range again, the actor would no longer follow the box.

src/systems/follow_actor/FollowActor.cc Outdated Show resolved Hide resolved
src/systems/follow_actor/FollowActor.cc Show resolved Hide resolved
@chapulina
Copy link
Contributor Author

one behavior of the follow actor system I noticed is that if the box moves of out range and back within range again, the actor would no longer follow the box.

That was actually a feature on ServiceSim. If the robot moved too fast, the human would get lost and stop following. It wouldn't restart following until the robot asked to be followed again.

Since this port doesn't have a transport interface, there's no way to get the actor to follow again. I realize this isn't very useful. So I changed the implementation so that the actor always follows if within range: e9588c2

@chapulina chapulina merged commit d7cd60a into master Jun 5, 2020
@chapulina chapulina deleted the follow_actor branch June 5, 2020 18:42
@adlarkin adlarkin mentioned this pull request Mar 12, 2021
11 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🔮 dome Ignition Dome
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants