Conversation
Signed-off-by: Michał Pełka <michal.pelka@robotec.ai>
jhanca-robotecai
left a comment
jhanca-robotecai
left a comment
There was a problem hiding this comment.
Please bump the version of the package.
peci1
left a comment
peci1
left a comment
There was a problem hiding this comment.
Hmm, I don't like this approach of making each field an array. However, given the fact that a service request (IMO) cannot be reused elsewhere, it is probably the best possible solution without going awkward (like specifying a standalone message type for the request part).
Please, see the inline comments.
| @@ -0,0 +1,46 @@ | |||
| # Spawn entities (a robot, other object) by name or URI | |||
| # Support for this interface is indicated through the SPAWNING_ARRAY value in GetSimulationFeatures. | |||
|
|
|||
There was a problem hiding this comment.
Add a comment about the array lengths. Do all of them need to be filled? Or are empty arrays supported? And what about unfinished arrays? (i.e. names[] has 5 elems but allow_renaming[] only 3).
| uint8 INVALID_POSE = 109 # initial_pose is invalid, such as when the quaternion is invalid or position | ||
| # exceeds simulator world bounds. | ||
|
|
||
| Result results[] |
There was a problem hiding this comment.
There should also be Result result as the overall result to keep the interface consistent. This should come with good specification on what should be put in it when only partial success is achieved. Maybe only restrict the error codes to OK (all succeeded), SOME_FAILED, FAILED?
| --- | ||
|
|
||
| # Additional result.result_code values for this service. Check result.error_message for further details. | ||
| uint8 NAME_NOT_UNIQUE = 101 # Given name is already taken by entity and allow_renaming is false. |
There was a problem hiding this comment.
Is it needed to repeat those here? Wouldn't it be better to link to SpawnEntity.srv?
I guess version bumps are done when releasing the package, so I wouldn't expect the bump to be a part of this PR. |
4 participants
Simulation interfaces can be used to populate scene with assets or props, depending on the scenario.
I would like to propose the way to ask underlying simulator to spawn multiple objects.
Motivation: