Taking first valid GPS message for time sync#21
Conversation
Georacer
left a comment
Georacer
left a comment
There was a problem hiding this comment.
Hello! Thank you for the PR!
A few questions:
- Why replace
windows-latestwith 2022? This sounds like a pretty old image. What is the benefit of doing that? - Could you squash your commits so that there is one commit for each concrete change? Right now there are too many little incremental changes.
I've also left an inline question too.
| 3. Build the plugin with | ||
| ```bash | ||
| docker build -o type=local,dest=artifacts . | ||
| docker build -o type=local,dest=artifacts --build-arg ADD_UNITS=ON . |
There was a problem hiding this comment.
We usually have units off, to not break people's older layouts. Do you have an argument to have this on by default?
|
I would prefer you split your commits like I did in this branch, where I copied your work: https://github.com/Georacer/plotjuggler-apbin-plugins/commits/pr3/gps_message/ I didn't use my branch to merge your work in, to give you a chance to have your signed commit in our APBin repo. But if that's not important to you, I can use my branch to bring in the new code. By the way, the Windows CI build now passes, many thanks! |
with units restore restore fix win build try fix fix plotjuggler multicore build fix linux build try again try fix fix run on windows 22 removed not needed windows files
faster build + fix take first valid gps msg fixes Refactor GPS time conversion and synchronization Refactor gps_to_unix_time function and update time synchronization logic to improve clarity and accuracy. Add support for tag-based builds in CI workflow Fix formatting issues in win_build.yml Update GitHub Actions workflow for PlotJuggler build fix time range
|
Hi, |
2 participants
Hey,
I added that the plugin uses the first valid GPS and not only the first GPS message in the log.
Had this problem recently that the Pixhawk booted up without GPS signal right after boot and then the log time sync didn't worked.
Changes: