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.

Sign up for GitHub

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

Jump to bottom

Surface tracker stability improvements #2025

Merged
merged 10 commits into from
Oct 19, 2020
Merged

Surface tracker stability improvements #2025

merged 10 commits into from
Oct 19, 2020

Conversation

papr
Copy link
Contributor

@papr papr commented Oct 9, 2020

papr added 10 commits October 9, 2020 14:26
@papr
Add NamedTuple for cache relevant detector parameters
254dd6e
@papr
_init_marker_cache: Better variable naming
3238564 
`previous_cache` is a dict, while `cache` was a list.
@papr
Always recalculate cache with current configuration
cd65880 
Fixes inverted legacy marker detection
@papr
MarkerDetectorMode: Use typing.NamedTuple
b16da3a
@papr
Add marker_detection_callable.from_detector
90b6431
@papr
Load cache-relevant params from cache
b8ba2fd
@papr
Add detector mode to cache-relevant params
f091cad
@papr
Use controller as storage for current configuration
6f1a131
@papr
Initialize background detector based on foreground detector
7cb79f7
@papr
Ensure consistent marker detector configuration
8fcdf5b 
Fixes #1806

The issue comes up if the plugins session settings conflict with the previous cache. Preferable behaviour should be:
- If there is no previous cache use the plugin's current settings
- If there is a previous cache
    - if cache is of correct version use cache config
    - if cache is of incorrect version use plugin's current settings
- If user changes plugin settings redetect with plugin's current settings
@papr papr added type: bug plugin: surface_tracker Issues and PRs related to the Surface Tracker plugin labels Oct 9, 2020
@papr papr requested a review from romanroibu October 9, 2020 15:14
@papr papr self-assigned this Oct 9, 2020
papr
papr commented Oct 9, 2020
View reviewed changes
Copy link
Contributor Author

@papr papr left a comment

Choose a reason for hiding this comment

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

Known issue: Crash on changing from legacy to apriltag marker detection

@papr
Copy link
Contributor Author

papr commented Oct 12, 2020

Not reproducible. Note from @s-westphal: The detection is not thread-safe. It is possible that I ran into a race condition.

@papr papr marked this pull request as ready for review October 13, 2020 07:50
@papr papr merged commit d4d0fbf into develop Oct 19, 2020
@papr papr deleted the surface_tracker_fixes branch October 19, 2020 08:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@romanroibu romanroibu romanroibu approved these changes

Assignees

@papr papr

Labels
plugin: surface_tracker Issues and PRs related to the Surface Tracker plugin type: bug
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@papr @romanroibu