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

Implement a diagnostic tool #154

Merged
merged 86 commits into from
Sep 5, 2023
Merged

Implement a diagnostic tool #154

merged 86 commits into from
Sep 5, 2023

Conversation

luca-della-vedova
Copy link
Member

New feature implementation

Implemented feature

Addresses most of #61, excluding the additional recommendations and the model issue (since lift / door models are not supported yet).
It is currently in draft and based on luca/drawing_editor because it needed some of the UI refactoring that went into that PR. However it also needs some of the refactoring that is in the fuel asset widget PR, so will move out of draft once that is finalized (and this widget is moved into a left panel rather than a floating window).

Implementation description

This PR adds the following features:

  • Checks for common types of issues, such as doors / lifts / docks with the same name, fiducials without a label and anchors that are very close but unconnected.
  • Suppressing issues by category.
  • Suppressing single issues.
  • Serializing / deserializing suppression data in the SiteProperties to make it persistent across sessions.

Example of the current behavior in the video below:

Screencast.from.2023-07-25.11-20-31.webm

The architecture of the issues finding / inserting is fully decentralized. Issues are entities spawned as children of the workspace and systems are responsible for registering them (mapping a UUID to the issue type through a new App trait) and spawning them, for now only as a response to validation events.

There is one slightly brittle aspect to the solution, which caused the odd system ordering. Specifically, since issues are entities it is necessary to despawn them when a new validation request is received, to avoid having duplicates.
At the same time, it would be a fair bit of code duplication to have every system be responsible for cleaning up its own issues, then respawning them afterwards, so I created a system that performs the cleanup. Right now the events are sent in Update, cleanup is performed in PostUpdate and population with new issues in PreUpdate, if the ordering was changed in a refactoring this feature might behave unexpectedly (i.e. the cleanup system clearing all the newly created issues if it runs after their creation and the issue list always being empty).

Signed-off-by: Luca Della Vedova <lucadv@intrinsic.ai>
Signed-off-by: Luca Della Vedova <lucadv@intrinsic.ai>
Signed-off-by: Luca Della Vedova <lucadv@intrinsic.ai>
Signed-off-by: Luca Della Vedova <lucadv@intrinsic.ai>
Signed-off-by: Luca Della Vedova <lucadv@intrinsic.ai>
Signed-off-by: Luca Della Vedova <lucadv@intrinsic.ai>
Signed-off-by: Luca Della Vedova <lucadv@intrinsic.ai>
Signed-off-by: Luca Della Vedova <lucadv@intrinsic.ai>
Signed-off-by: Luca Della Vedova <lucadv@intrinsic.ai>
Signed-off-by: Luca Della Vedova <lucadv@intrinsic.ai>
Signed-off-by: Luca Della Vedova <lucadv@intrinsic.ai>
Signed-off-by: Luca Della Vedova <lucadv@intrinsic.ai>
Signed-off-by: Luca Della Vedova <lucadv@intrinsic.ai>
Signed-off-by: Luca Della Vedova <lucadv@intrinsic.ai>
Signed-off-by: Luca Della Vedova <lucadv@intrinsic.ai>
Signed-off-by: Luca Della Vedova <lucadv@intrinsic.ai>
Signed-off-by: Luca Della Vedova <lucadv@intrinsic.ai>
Signed-off-by: Luca Della Vedova <lucadv@intrinsic.ai>
Signed-off-by: Luca Della Vedova <lucadv@intrinsic.ai>
Signed-off-by: Luca Della Vedova <lucadv@intrinsic.ai>
Signed-off-by: Luca Della Vedova <lucadv@intrinsic.ai>
Signed-off-by: Luca Della Vedova <lucadv@intrinsic.ai>
Signed-off-by: Luca Della Vedova <lucadv@intrinsic.ai>
Signed-off-by: Luca Della Vedova <lucadv@intrinsic.ai>
Signed-off-by: Luca Della Vedova <lucadv@intrinsic.ai>
Signed-off-by: Luca Della Vedova <lucadv@intrinsic.ai>
Signed-off-by: Luca Della Vedova <lucadv@intrinsic.ai>
Signed-off-by: Luca Della Vedova <lucadv@intrinsic.ai>
Signed-off-by: Luca Della Vedova <lucadv@intrinsic.ai>
Signed-off-by: Luca Della Vedova <lucadv@intrinsic.ai>
Signed-off-by: Luca Della Vedova <lucadv@intrinsic.ai>
Signed-off-by: Luca Della Vedova <lucadv@intrinsic.ai>
Signed-off-by: Luca Della Vedova <lucadv@intrinsic.ai>
Signed-off-by: Luca Della Vedova <lucadv@intrinsic.ai>
Signed-off-by: Luca Della Vedova <lucadv@intrinsic.ai>
Signed-off-by: Luca Della Vedova <lucadv@intrinsic.ai>
Signed-off-by: Luca Della Vedova <lucadv@intrinsic.ai>
Base automatically changed from luca/drawing_editor to main August 14, 2023 09:36
Signed-off-by: Luca Della Vedova <lucadv@intrinsic.ai>
Signed-off-by: Luca Della Vedova <lucadv@intrinsic.ai>
Signed-off-by: Luca Della Vedova <lucadv@intrinsic.ai>
Signed-off-by: Luca Della Vedova <lucadv@intrinsic.ai>
Signed-off-by: Luca Della Vedova <lucadv@intrinsic.ai>
Signed-off-by: Luca Della Vedova <lucadv@intrinsic.ai>
Signed-off-by: Luca Della Vedova <lucadv@intrinsic.ai>
@luca-della-vedova luca-della-vedova marked this pull request as ready for review August 29, 2023 06:25
@luca-della-vedova
Copy link
Member Author

Sadly there is a fair bit of noise coming from cargo fmt since let else was just stabilized, hopefully most of these changes should be straightforward

luca-della-vedova and others added 9 commits August 29, 2023 14:32
Signed-off-by: Luca Della Vedova <lucadv@intrinsic.ai>
Signed-off-by: Luca Della Vedova <lucadv@intrinsic.ai>
Signed-off-by: Luca Della Vedova <lucadv@intrinsic.ai>
Signed-off-by: Luca Della Vedova <lucadv@intrinsic.ai>
Signed-off-by: Luca Della Vedova <lucadv@intrinsic.ai>
Signed-off-by: Michael X. Grey <grey@openrobotics.org>
Signed-off-by: Michael X. Grey <grey@openrobotics.org>
Signed-off-by: Michael X. Grey <grey@openrobotics.org>
Copy link
Collaborator

@mxgrey mxgrey left a comment

Choose a reason for hiding this comment

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

I made some tweaks; one important one to make sure that saving/loading works correctly.

This feature is awesome, and I look forward to adding more warnings to it over time.

@mxgrey mxgrey merged commit fbc5fbf into main Sep 5, 2023
5 checks passed
@mxgrey mxgrey deleted the luca/diagnostic_tool branch September 5, 2023 06:15
@luca-della-vedova luca-della-vedova linked an issue Sep 6, 2023 that may be closed by this pull request
5 tasks
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.

Audit / Diagnostic tool
2 participants