Skip to content

Conversation

mjcarroll
Copy link
Contributor

Adds the ability to score artifacts by name at their true positions using the ArtifactValidationTool.

This will only work from the ignition side, as it requires the SDF ground truth of the artifact positions.

To score an artifact, use the service call:

ign service -s /artifact/score \
--reqtype ignition.msgs.StringMsg \
--reptype ignition.msgs.StringMsg \
--timeout 1000 \
--req 'data: "backpack_3"'

The validator tool is currently still subject to the comms limitations of the environment, so distant artifacts may be "unscorable" due to radio outages.

Signed-off-by: Michael Carroll <michael@openrobotics.org>
Signed-off-by: Michael Carroll <michael@openrobotics.org>
@iche033
Copy link
Contributor

iche033 commented Mar 17, 2021

I used this for testing the finals practice worlds. Works great.

I just had to cycle through all the artifacts first to make sure they are loaded and test scoring artifacts after moving the validator to the artifact_origin so that it's in comms range.

One thing I did notice is that in finals practice world 03, the comms client had trouble binding / registering on launch:

this->dataPtr->client->Bind(&ArtifactValidatorPrivate::OnArtifactAck,

I just manually added a hack to initialize the client only after _info.simTime is greater than 15 seconds. I think it could be due to this world taking longer to load on my laptop.


/////////////////////////////////////////////////
bool ArtifactValidatorPrivate::OnScore(const ignition::msgs::StringMsg& _req,
ignition::msgs::StringMsg& _rep)
Copy link
Contributor

Choose a reason for hiding this comment

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

_rep is unused

@mjcarroll
Copy link
Contributor Author

I just had to cycle through all the artifacts first to make sure they are loaded and test scoring artifacts after moving the validator to the artifact_origin so that it's in comms range.

Hmm, I was under the impression that this shouldn't be necessary anymore, let me double check that.

I just manually added a hack to initialize the client only after _info.simTime is greater than 15 seconds. I think it could be due to this world taking longer to load on my laptop.

I originally had it during the constructor, which was always too early. My understanding is that all systems should have been Configure-d by the time the first PostUpdate hits, so it shouldn't have been an issue. I wonder if there is some other race condition in there?

@iche033
Copy link
Contributor

iche033 commented Mar 18, 2021

this is what I see without delaying the Bind() function:

[validator] CommsClient::Register: Problem registering with broker
[CommsClient] Retrying register..
[validator] CommsClient::Register: Problem registering with broker
[CommsClient] Retrying register..
[validator] CommsClient::Register: Problem registering with broker
[CommsClient] Retrying register..
[validator] CommsClient::Register: Problem registering with broker
[CommsClient] Validation service not available, invalid address or model not available
[validator] Bind() error: Trying to bind before communications are enabled!
[Msg] Moving to: artifact_origin
[Dbg] [ArtifactValidator.cc:175] Result: 1 1
[Err] [UserCommands.cc:931] Unable to update the pose for entity id:[0], name[validator]
[Dbg] [CommsBrokerPlugin.cc:218] Using [visibility_range] comms model
[Msg] Starting SubT comms broker

Looks like the SubT comms broker is started after the ArtifiactValidator. It could be because the CommsBroker plugin is an ign-launch plugin while the ArtifactValidator plugin is an ign-gazebo system

@zbynekwinkler
Copy link

I just had to cycle through all the artifacts first to make sure they are loaded and test scoring artifacts after moving the validator to the artifact_origin so that it's in comms range.

Hmm, I was under the impression that this shouldn't be necessary anymore, let me double check that.

I just manually added a hack to initialize the client only after _info.simTime is greater than 15 seconds. I think it could be due to this world taking longer to load on my laptop.

I originally had it during the constructor, which was always too early. My understanding is that all systems should have been Configure-d by the time the first PostUpdate hits, so it shouldn't have been an issue. I wonder if there is some other race condition in there?

AFAIU there is no way to tell the system is "ready-to-go". Se our discussion wrt this in #412. What we ended up doing is watching "topic churn" - repeatedly listing all available ROS topics and waiting for such a time that this list is no longer growing plus a buffer of couple seconds. It's a pity something has not been designed into the system that would allow to determine with certainty that everything is done loading and is ready to go.

@mjcarroll
Copy link
Contributor Author

As far as the artifacts loading, I was mistaken, that logic remains the same in GameLogicPlugin. You still have to iterate enough for all of the artifacts to be loaded in GameLogicPlugin (although ArtifactValidator is aware of all of them without concern for levels)

In terms of the sequencing issue with comms, this is purely on the ignition side without any ROS interfaces, so checking the ROS topics wouldn't reveal anything. I think that what @iche033 said about a system plugin loaded via ign-gazebo versus a system plugin loaded via ign-plugin is probably the most likely candidate for a race condition or sequencing error.

Each ign-gazebo system should handle all configuration during the System::Configure step, so by the time the first PostUpdate step is reached, everything should be set up.

If the comms client doesn't connect, it will retry every
100 ms of sim time.

Signed-off-by: Michael Carroll <michael@openrobotics.org>
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.

the retry method works for me and I can now score artifacts in finals practice 03 world.

@mjcarroll mjcarroll merged commit d20782e into master Apr 1, 2021
@mjcarroll mjcarroll deleted the validate_artifact_scoring branch April 1, 2021 23:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants