fix: CSV export missing values for quaternion and RPY series#1290
Closed
pradhankukiran wants to merge 3 commits intoPlotJuggler:mainfrom
Closed
fix: CSV export missing values for quaternion and RPY series#1290pradhankukiran wants to merge 3 commits intoPlotJuggler:mainfrom
pradhankukiran wants to merge 3 commits intoPlotJuggler:mainfrom
Conversation
Collaborator
|
CI is red sir |
Collaborator
|
nevermind, that was my fault. for the PR, actually you should return NaN if you want plotJuggler to "skip it". do not return 0.0 |
Contributor
Author
|
@facontidavide done, returning NaN instead of 0.0 now. thanks for finding it out. |
Collaborator
|
this plugin has been retired |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #1137 - quaternion and roll/pitch/yaw columns appear in CSV export with headers but no values.
Four root causes identified and fixed:
Off-by-one in index initialization (
publisher_csv.cpp:291):indices[i] = index + 1skipped the first in-range sample of every series. Changed toindices[i] = index, consistent withgenerateStatisticsCSV().Machine-epsilon timestamp matching (
publisher_csv.cpp:328): Values from different series were merged into a CSV row only when timestamps matched withinstd::numeric_limits<double>::epsilon()(~2.2e-16). Real sensor data has larger floating-point drift between channels, causing cells to go blank. Replaced with an adaptive tolerance (min_dt * 0.5) derived from the minimum sample period across all exported series — mathematically guaranteed to never merge distinct adjacent samples.NaN values stalling index advancement (
publisher_csv.cpp:344-348): The index only advanced when a value was not NaN, making it impossible to distinguish "no data at this timestamp" from "data is NaN at this timestamp." Added arow_usedboolean vector to track participation explicitly, so indices advance correctly regardless ofNaN status.
No zero-norm quaternion guard (
special_messages.cpp,quaternion_to_rpy.cpp): A zero quaternion(0,0,0,0)caused1.0 / sqrt(0) = inf, producing NaN for all RPY values, which then triggered 3rd bug. Added an early return of(0,0,0)RPY whenquat_norm2 < 1e-10.