fix: clock type mismatch and duration cast unit in test_timer_t#1020
Closed
aki1770-del wants to merge 1 commit into
Closed
fix: clock type mismatch and duration cast unit in test_timer_t#1020aki1770-del wants to merge 1 commit into
aki1770-del wants to merge 1 commit into
Conversation
Two bugs in test/common/include/common/test_timer.hpp:
1. start member was declared as system_clock::time_point but initialized
via high_resolution_clock::now() in both constructors. On platforms
where high_resolution_clock is not an alias for system_clock (Android
x86_64 with Clang, MSVC) this fails with:
no matching constructor for initialization of
'std::chrono::system_clock::time_point'
Fix: change member type to high_resolution_clock::time_point,
consistent with the initialization expression.
2. has_elapsed() cast the elapsed duration to std::chrono::seconds before
comparing against target (std::chrono::milliseconds). For sub-second
timers the cast always produces seconds(0), causing the timer to fire
immediately regardless of target.
Fix: use duration_cast<std::chrono::milliseconds> to match target units.
Fixes COVESA#821
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
2 participants
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 #821
Two bugs in
test/common/include/common/test_timer.hpp:Bug 1 — Clock type mismatch (compile error on Android/Clang)
startis declared asstd::chrono::system_clock::time_pointbut initializedvia
std::chrono::high_resolution_clock::now()in both constructors.On platforms where
high_resolution_clockis not an alias forsystem_clock(Android x86_64 with Clang, MSVC), this fails with:
Fix: change the member type to
std::chrono::high_resolution_clock::time_point,consistent with the initialization expression already present.
Bug 2 — Duration cast to wrong unit (logic error)
has_elapsed()casts the elapsed duration tostd::chrono::secondsbeforecomparing against
target, which isstd::chrono::milliseconds. For sub-secondtimers the cast always produces
seconds(0), causing the timer to fire immediatelyregardless of the target interval.
Fix: change
duration_cast<std::chrono::seconds>toduration_cast<std::chrono::milliseconds>.Both fixes are single-line changes in the same header. No production code
is affected. No new includes or dependencies.