Skip to content
This repository has been archived by the owner on Jun 4, 2024. It is now read-only.

Clear topic subscription errors when new messages are received #3451

Merged
merged 2 commits into from
May 25, 2022

Conversation

foxymiles
Copy link
Contributor

@foxymiles foxymiles commented May 25, 2022

User-Facing Changes
None

Description
Currently once we encounter a subscription error on a topic on a ROS connection we display a player error but we don't clear this error once the subscription to that topic is working again. The solution here is to clear the error once we successfully receive a message on that topic again.

Fixes #3293

@jhurliman
Copy link
Contributor

Would the ROS2 player benefit from the same fix?

@foxymiles
Copy link
Contributor Author

Would the ROS2 player benefit from the same fix?

Yeah looks like it. I'll add it there as well.

@defunctzombie
Copy link
Contributor

Since there can be multiple publishers to a single topic - what happens when a subscriber to one of the publishers fails but the other is working? Would we still surface an error to the user? If we have a subscription id we should include that in the error key so we can clear only the errors for the subscription that is working again.

@foxymiles
Copy link
Contributor Author

foxymiles commented Jun 6, 2022

Good question. Looks like right now we don't know the id of the publisher that caused the error?

subscription.on("error", (error) => {
this._addProblem(`subscribe:${topicName}`, {
severity: "warn",
message: `Topic subscription error for "${topicName}"`,
tip: `The subscription to "${topicName}" will be automatically re-established`,
error,
});
});

@defunctzombie
Copy link
Contributor

@foxymiles I see yes the subscription is for any publisher. We'd need to update our ros1 library to provide that additional information in the error. Something we'll have to stash away for later.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Development

Successfully merging this pull request may close these issues.

When a connection to a topic is reestablished, it should clear the errors
3 participants