small fixes for release#2019
Open
paul-nechifor wants to merge 1 commit intodevfrom
Open
Conversation
Contributor
Greptile SummaryThis PR makes two code fixes (migrating
Confidence Score: 1/5Not safe to merge — The lidar module was committed with an unresolved merge conflict: the
Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A["Incoming lidar item (T)"] --> B{use_system_time?}
B -- Yes --> C["item.ts = now()"]
C --> R["return item"]
B -- No --> D{calibrated?}
D -- No --> E["compare item.ts vs prev_raw"]
E --> F["update prev_raw, n_seen"]
F --> G{prev_good != None and item.ts <= prev_good?}
D -- Yes --> G
G -- Yes --> H["item.ts = prev_good + default_period (stale stamp repair)"]
G -- No --> I["prev_good = item.ts"]
H --> I
I --> J{not calibrated and n_seen >= calibration_frames?}
J -- Yes --> K["use_system_time = True, log warning"]
J -- No --> R
K --> R
Reviews (2): Last reviewed commit: "exclude unitree-dds" | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Comment on lines
99
to
103
| def _repair(item: T) -> T: | ||
| if prev_good[0] is not None and item.ts <= prev_good[0]: | ||
| old = item.ts | ||
| item.ts = prev_good[0] + default_period | ||
| logger.warning("repair_stale_ts: stale stamp %.6f → %.6f", old, item.ts) | ||
| prev_good[0] = item.ts | ||
| return item |
Contributor
There was a problem hiding this comment.
The logger.warning that recorded both the original stale stamp and the repaired value was removed. Without it, there is no way to detect (in logs or metrics) how often the stale-stamp bug fires in production, making it harder to notice if the condition starts occurring more frequently or signals a new firmware regression. If the intent is to reduce log noise, consider using logger.debug instead of dropping the diagnostic entirely.
❌ 13 Tests Failed:
View the top 3 failed test(s) by shortest run time
To view more test analytics, go to the Test Analytics Dashboard |
paul-nechifor
force-pushed
the
paul/fix/release-fixes
branch
from
0991b61 to
7f56518
Compare
Comment on lines
108
to
138
| @@ -119,9 +120,7 @@ def _repair(item: T) -> T: | |||
| n_seen += 1 | |||
|
|
|||
| if prev_good is not None and item.ts <= prev_good: | |||
| old = item.ts | |||
| item.ts = prev_good + default_period | |||
| logger.warning("repair_stale_ts: stale stamp %.6f → %.6f", old, item.ts) | |||
|
|
|||
| prev_good = item.ts | |||
|
|
|||
| @@ -133,6 +132,9 @@ def _repair(item: T) -> T: | |||
| calibration_frames, | |||
| ) | |||
|
|
|||
| if prev_good[0] is not None and item.ts <= prev_good[0]: | |||
| item.ts = prev_good[0] + default_period | |||
| prev_good[0] = item.ts | |||
| return item | |||
Contributor
There was a problem hiding this comment.
Line 108 contains a raw <<<<<<< HEAD conflict marker, so Python will raise a SyntaxError when this module is imported, breaking every code path that depends on repair_stale_ts. Beyond the marker, the conflict also left two mutually incompatible representations of prev_good in the same function body: the earlier code treats it as a float | None scalar (prev_good = item.ts), while lines 135–137 treat it as an indexable container (prev_good[0]). Even after removing the conflict marker, prev_good[0] on a float | None would raise a TypeError at runtime.
The file needs a clean resolution that picks one approach (scalar + nonlocal, or a single-element list for implicit closure mutation) and applies it consistently throughout _repair.
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
1 participant
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.
Problem
Closes DIM-XXX
Solution
Breaking Changes
How to Test
Contributor License Agreement