Skip to content

Log breadcrumb events #555

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Merged
merged 5 commits into from
Aug 31, 2020
Merged

Log breadcrumb events #555

merged 5 commits into from
Aug 31, 2020

Conversation

nkoenig
Copy link
Contributor

@nkoenig nkoenig commented Aug 18, 2020

This adds breadcrumb deployments to the event log. This will capture breadcrumb deployment attempts.

Signed-off-by: Nate Koenig nate@openrobotics.org

Signed-off-by: Nate Koenig <nate@openrobotics.org>
@nkoenig nkoenig requested review from azeey and caguero August 20, 2020 12:15
Signed-off-by: Nate Koenig <nate@openrobotics.org>
@nkoenig nkoenig removed the request for review from azeey August 25, 2020 16:05
Copy link
Contributor

@acschang acschang left a comment

Choose a reason for hiding this comment

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

The current functionality works for me.

It might make sense to add a max_breadcrumb_deployments event corresponding to a failed breadcrumb_deploy event that exceeds the limit of breadcrumbs deployed similar to how rock falls beyond the limit are recorded.

Nate Koenig added 2 commits August 27, 2020 06:00
Signed-off-by: Nate Koenig <nate@openrobotics.org>
Signed-off-by: Nate Koenig <nate@openrobotics.org>
@nkoenig
Copy link
Contributor Author

nkoenig commented Aug 27, 2020

The current functionality works for me.

It might make sense to add a max_breadcrumb_deployments event corresponding to a failed breadcrumb_deploy event that exceeds the limit of breadcrumbs deployed similar to how rock falls beyond the limit are recorded.

Add max deployment check in 8e74ef0

Signed-off-by: Nate Koenig <nate@openrobotics.org>
Copy link
Contributor

@caguero caguero left a comment

Choose a reason for hiding this comment

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

It works for me as expected as well.

@nkoenig nkoenig merged commit 2c3f6a9 into master Aug 31, 2020
@nkoenig nkoenig deleted the breadcrumb_log_event branch December 10, 2020 22:31
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants