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

Add parameters system #1280

Closed
wants to merge 11 commits into from
Closed

Conversation

ivanpauno
Copy link
Contributor

@ivanpauno ivanpauno commented Jan 11, 2022

🎉 New feature

Connected to gazebosim/gz-msgs#211.
Hints of how to review the PR: #1280 (comment).

Summary

This PR adds:

  • A parameters system:
    • Other systems can declare parameter using the ignition::gazebo::DeclareParameter() helper function.
    • Parameters can be introspected or set from external processes through three service provided by the Parameters system:
      • /world/<world_name>/list_parameters service: List available parameters names and types.
      • /world/<world_name>/get_parameter service: Get the type and value of a parameter.
      • /world/<world_name>/set_parameter service: Set a parameter: parameter name, value and type need to be provided.
    • Parameters types are protobuf messages.
    • Parameters are automatically associated to a component in the entity component manager, there's no separate "register" of parameters. When the parameter is set, the associated component is updated.
      The component data type must be a protobuf message to be declared as a parameter.
  • A ign param command line tool. A simple cli tool to interact with parameters.
    Though interacting with parameters using ign service is possible, it's not as easy as parameter values are serialized.

Summary of what's needed to declare and use a parameter can be seen in the wheel slip demo branch: diff.

Test it

I haven't written automated tests yet, but there's a demo using parameters in the wheel slip plugin:

Steps to run the demo:

  • Build from source ignition edifice checking out the two branches specified above (I can provide PRs for another version if needed).
  • Download the following sdf demo world: https://gist.github.com/ivanpauno/2d7ad7bb4ea8ca88aaaa89458999cb0f
    wget https://gist.githubusercontent.com/ivanpauno/2d7ad7bb4ea8ca88aaaa89458999cb0f/raw/3710fb389f9c841e4992835a4dc1dd5ab933cefd/wheel_slip_parameters_world.xml
    
  • Launch the downloaded world in ignition gazebo, in a terminal with the previous workspace sourced.
    ign gazebo ./wheel_slip_parameters_world.xml
    
  • In another terminal with the workspace sourced:
    • List available parameters:

      ign param -l
      
      Expected output

      systems.wheel_slip.trisphere_cycle1.wheel_front [ignition.msgs.WheelSlipParameters]
      systems.wheel_slip.trisphere_cycle1.wheel_rear_right [ignition.msgs.WheelSlipParameters]
      systems.wheel_slip.trisphere_cycle0.wheel_rear_right [ignition.msgs.WheelSlipParameters]
      systems.wheel_slip.trisphere_cycle1.wheel_rear_left [ignition.msgs.WheelSlipParameters]
      systems.wheel_slip.trisphere_cycle0.wheel_rear_left [ignition.msgs.WheelSlipParameters]
      systems.wheel_slip.trisphere_cycle0.wheel_front [ignition.msgs.WheelSlipParameters]

    • Get the systems.wheel_slip.trisphere_cycle1.wheel_front parameter:

      ign param --get -n systems.wheel_slip.trisphere_cycle1.wheel_front
      
      Expected output

      Getting parameter [systems.wheel_slip.trisphere_cycle1.wheel_front] for world [wheel_slip]...
      Parameter type [ignition.msgs.WheelSlipParameters]

      ------------------------------------------------
      slip_compliance_lateral: 1
      slip_compliance_longitudinal: 1
      ------------------------------------------------

    • Set the systems.wheel_slip.trisphere_cycle1.wheel_front parameter to a different value:

      ign param --set -n systems.wheel_slip.trisphere_cycle1.wheel_front -t ignition.msgs.WheelSlipParameters -m "
        slip_compliance_lateral: 1
        slip_compliance_longitudinal: 2"
      
      Expected output

      Setting parameter [systems.wheel_slip.trisphere_cycle1.wheel_front] for world [wheel_slip]...

      Parameter successfully set!

    • Get the systems.wheel_slip.trisphere_cycle1.wheel_front parameter again, its value should have changed:

      ign param --get -n systems.wheel_slip.trisphere_cycle1.wheel_front
      
      Expected output

      Getting parameter [systems.wheel_slip.trisphere_cycle1.wheel_front] for world [wheel_slip]...
      Parameter type [ignition.msgs.WheelSlipParameters]

      ------------------------------------------------
      slip_compliance_lateral: 1
      slip_compliance_longitudinal: 2
      ------------------------------------------------

Checklist

  • Signed all commits for DCO
  • Added tests
  • Added example and/or tutorial
  • Updated documentation (as needed)
  • Updated migration guide (as needed)
  • codecheck passed (See contributing)
  • All tests passed (See test coverage)
  • While waiting for a review on your PR, please help review another open pull request to support the maintainers

Note to maintainers: Remember to use Squash-Merge and edit the commit message to match the pull request summary while retaining Signed-off-by messages.

@ivanpauno ivanpauno added the enhancement New feature or request label Jan 11, 2022
@ivanpauno ivanpauno self-assigned this Jan 11, 2022
@github-actions github-actions bot added the 🏢 edifice Ignition Edifice label Jan 11, 2022
Signed-off-by: Ivan Santiago Paunovic <ivanpauno@ekumenlabs.com>
Signed-off-by: Ivan Santiago Paunovic <ivanpauno@ekumenlabs.com>
Signed-off-by: Ivan Santiago Paunovic <ivanpauno@ekumenlabs.com>
Signed-off-by: Ivan Santiago Paunovic <ivanpauno@ekumenlabs.com>
…stry with mutex

Signed-off-by: Ivan Santiago Paunovic <ivanpauno@ekumenlabs.com>
Signed-off-by: Ivan Santiago Paunovic <ivanpauno@ekumenlabs.com>
Signed-off-by: Ivan Santiago Paunovic <ivanpauno@ekumenlabs.com>
Signed-off-by: Ivan Santiago Paunovic <ivanpauno@ekumenlabs.com>
Signed-off-by: Ivan Santiago Paunovic <ivanpauno@ekumenlabs.com>
Signed-off-by: Ivan Santiago Paunovic <ivanpauno@ekumenlabs.com>
Signed-off-by: Ivan Santiago Paunovic <ivanpauno@ekumenlabs.com>
@ivanpauno ivanpauno force-pushed the ivanpauno/parameters-impl-draft branch from dc742f3 to 1ca4743 Compare January 11, 2022 22:22
@ivanpauno
Copy link
Contributor Author

Hints of how to review this:

  • Look at the demo of how to use parameters in the wheel slip plugin to have an idea of how they're used: diff.
  • Look at the Test it instructions in the PR description to have an idea of how the user facing interface is.
    • It's not needed to actually reproduce all the instructions, just reading the commands and their outputs is fine.
  • Look at changes in src/systemsand the file include/ignition/gazebo/components/ParameterDeclarationCmd.hh.
    Looking at the message definitions added in Add message definitions for the parameter system gz-msgs#211 at the same time may be helpful.
  • Look at include/ignition/gazebo/Parameters.hh.
  • Look at changes in src/cmd.
  • Look at the remaining changes.

@chapulina chapulina added the needs upstream release Blocked by a release of an upstream library label Jan 11, 2022
@chapulina
Copy link
Contributor

@ivanpauno , Edifice will EOL next week, so this PR can't target ign-gazebo5 anymore. I believe we won't merge it in its current form anyway, right? Do you want to keep it open for visibility, or is it ok to close it?

@ivanpauno
Copy link
Contributor Author

It's fine to close it, should the new PRs target ign-gazebo6?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🏢 edifice Ignition Edifice enhancement New feature or request needs upstream release Blocked by a release of an upstream library
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants