Add elevator system

[nlamprian]

Compared to the classic gazebo elevator implementation:

• the system has an equivalent interface - int command to send the elevator to a given floor
• the model has doors attached to the floors (not the cabin), and the system manages them appropriately
• more features in both the model and system that aim to more closely approximate a real elevator

Tested in Dome. Let me know to rebase if you think it will work in Citadel.

[nkoenig]

Thank you for the contribution. Overall this PR looks great. The only problem is that we would like to avoid the inclusion of boost due to dependency and cross-platform issues.

I've found afsm which claims that migration from boost::msm is easy. Do you think you could put afsm in a vender sub-directory in your plugin and swap out boost for afsm?

[nlamprian]

Sure, I understand. Looks like an easy fix, I'll give it a try.

Just to be clear, you are suggesting that I copy the repo in the vender subdirectory, right? (and not configure it, for example, as a cmake external project or a git submodule)

[nkoenig]

Just to be clear, you are suggesting that I copy the repo in the vender subdirectory, right? (and not configure it, for example, as a cmake external project or a git submodule)

Yeah, that's correct.

[chapulina]

Looks like an easy fix, I'll give it a try.

Hi @nlamprian , are you planning to follow up on this PR? Thanks!

[nlamprian]

Yes, I just need to find the time. Last time I tried I got stuck trying to configure afsm in the project. The change is not trivial after all.

[chapulina]

@nlamprian , how about moving this plugin to the examples folder? The requirements for plugins there are looser than the official ones because we don't package and ship them. I think it would be ok to keep boost as a dependency if the plugin was there.

[nlamprian]

It's been that long, huh? 😀

I don't mind. Either we go with the example and I do it now, or we stay with the system and I do it within the month.

[chapulina]

Either we go with the example and I do it now, or we stay with the system and I do it within the month.

I'll leave that up to you 😉 Thanks!

[chapulina]

Hi @nlamprian , any updates? Let us know how we can help. Thanks!

[matosinho]

@nlamprian I'm interested in this plugin as well, so if there is a way for me to contribute, let me know.

[nlamprian]

Sorry for the broken promises and for keeping this on hold all this time.

Back in August, after I upgraded my setup, the system wasn't working as intended. The lidars were keeping the doors open. Their data didn't make sense and I couldn't figure out what was wrong soon enough.

Regardless, I upgraded again and things seem to be in order. So, I'll get right to it!

Btw I trying to visualize the lidars and gazebo segfaults in VisualizeLidar::Update with "Address not mapped to object". I copied the configuration from the respective example (which works). Any idea what might be wrong?

[chapulina]

Sorry for the broken promises and for keeping this on hold all this time.

No worries, we're all busy 😉

Btw I trying to visualize the lidars and gazebo segfaults in VisualizeLidar::Update with "Address not mapped to object".

I'm not able to reproduce using gpu_lidar_sensor.sdf. Would you mind opening an issue with steps to reproduce? Thanks!

[nlamprian]

I'm done with the migration, but the system doesn't work. I'll need to do some debugging.

I pushed a commit to get your opinion. I added an argument in gz_add_system for specifying target includes. This is necessary to pickup the headers under the vender sub-directory.

I opened an issue for the bug in #1202

[nlamprian]

Migration is complete. I hope I made it worth the wait.

The dependencies (afsm, metapushkin) are under the vender directory as requested. I grabbed the headers and license out of the repos.

Everything else stays the same.

[codecov]

Codecov Report

Merging #535 (a690342) into ign-gazebo3 (e032208) will increase coverage by 0.48%.
The diff coverage is 88.83%.

@@               Coverage Diff               @@
##           ign-gazebo3     #535      +/-   ##
===============================================
+ Coverage        77.19%   77.67%   +0.48%     
===============================================
  Files              227      248      +21     
  Lines            13578    14223     +645     
===============================================
+ Hits             10481    11048     +567     
- Misses            3097     3175      +78     

Impacted Files	Coverage Δ
src/systems/elevator/ElevatorCommonPrivate.hh	0.00% <0.00%> (ø)
src/systems/elevator/ElevatorStateMachine.hh	0.00% <0.00%> (ø)
src/systems/elevator/utils/DoorTimer.hh	0.00% <0.00%> (ø)
src/systems/elevator/utils/JointMonitor.hh	0.00% <0.00%> (ø)
...elevator/state_machine/ElevatorStateMachineImpl.hh	73.52% <73.52%> (ø)
.../systems/elevator/vender/afsm/include/afsm/fsm.hpp	78.78% <78.78%> (ø)
src/systems/elevator/Elevator.cc	86.52% <86.52%> (ø)
...vator/vender/afsm/include/afsm/detail/observer.hpp	87.50% <87.50%> (ø)
...evator/vender/afsm/include/afsm/detail/actions.hpp	90.24% <90.24%> (ø)
src/systems/elevator/state_machine/ActionsImpl.hh	92.30% <92.30%> (ø)
... and 12 more

---

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 e032208...a690342. Read the comment docs.

[chapulina]

Thanks again for the PR, it has a lot of functionality! Unfortunately the big ones take a lot of effort to review, so we're going slowly. I did a first high-level review now.

We're preparing the final ign-gazebo4 releases now, since Dome will EOL in a few days. I think we won't have time to finish iterating on this PR by then 😕 So it will need to be retargeted at another branch. You asked above if this could work with Citadel, and I think that should be reasonable, but I'll leave it up to whatever is easier for you.

Thanks again!

[nlamprian]

I backported the branch to citadel and tested it successfully. Now, how do you want this done? New PR / How do we proceed with the review?

[chapulina]

I backported the branch to citadel and tested it successfully. Now, how do you want this done? New PR / How do we proceed with the review?

Thanks! We can keep this PR, you just need to rebase onto ign-gazebo3 and change the PR base branch.

[nkoenig]

This works for me. I have a few style changes that I'll make in a separate PR.

[osrf-triage]

This pull request has been mentioned on Gazebo Community. There might be relevant details there:

https://community.gazebosim.org/t/new-ignition-releases-2022-03-01-citadel-edifice-fortress/1313/1

[osrf-triage]

This pull request has been mentioned on Gazebo Community. There might be relevant details there:

https://community.gazebosim.org/t/new-ignition-releases-2022-04-13-fortress-edifice/1367/1