Skip to content
New issue

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

Sign up for GitHub

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

Already on GitHub? Sign in to your account

Jump to bottom

Add Os::RawTime OSAL implementation, refactor Os::IntervalTimer #2923

Merged
merged 22 commits into from
Oct 14, 2024
Merged

Add Os::RawTime OSAL implementation, refactor Os::IntervalTimer #2923

merged 22 commits into from
Oct 14, 2024

Conversation

thomas-bc
Copy link
Collaborator

@thomas-bc thomas-bc commented Oct 8, 2024 •
edited
Loading

Related Issue(s) #2726 #2740
Has Unit Tests (y/n)
Documentation Included (y/n)

Change Description

Implements #2726 and #2740

Rationale

See tickets above

github-advanced-security[bot]
github-advanced-security bot found potential problems Oct 8, 2024
View reviewed changes
Copy link

@github-advanced-security github-advanced-security bot left a comment

Choose a reason for hiding this comment

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

CodeQL found more than 20 potential problems in the proposed changes. Check the Files changed tab for more details.

github-advanced-security[bot]
github-advanced-security bot found potential problems Oct 8, 2024
View reviewed changes
Fw/Time/TimeInterval.cpp
return *this;
}

bool TimeInterval::operator==(const TimeInterval& other) const {

Check notice

Code scanning / CodeQL

Use of basic integral type Note

operator== uses the basic integral type bool rather than a typedef with size and signedness.
Fw/Time/TimeInterval.cpp
return (TimeInterval::compare(*this,other) == EQ);
}

bool TimeInterval::operator!=(const TimeInterval& other) const {

Check notice

Code scanning / CodeQL

Use of basic integral type Note

operator!= uses the basic integral type bool rather than a typedef with size and signedness.
Fw/Time/TimeInterval.cpp
return (TimeInterval::compare(*this,other) != EQ);
}

bool TimeInterval::operator>(const TimeInterval& other) const {

Check notice

Code scanning / CodeQL

Use of basic integral type Note

operator> uses the basic integral type bool rather than a typedef with size and signedness.
Fw/Time/TimeInterval.cpp
return (TimeInterval::compare(*this,other) == GT);
}

bool TimeInterval::operator<(const TimeInterval& other) const {

Check notice

Code scanning / CodeQL

Use of basic integral type Note

operator< uses the basic integral type bool rather than a typedef with size and signedness.
Fw/Time/TimeInterval.cpp
return (TimeInterval::compare(*this,other) == LT);
}

bool TimeInterval::operator>=(const TimeInterval& other) const {

Check notice

Code scanning / CodeQL

Use of basic integral type Note

operator>= uses the basic integral type bool rather than a typedef with size and signedness.
Os/RawTime.hpp Fixed Show resolved Hide resolved
Svc/ActiveRateGroup/ActiveRateGroup.cpp
@@ -52,12 +52,12 @@
this->log_DIAGNOSTIC_RateGroupStarted();
}

void ActiveRateGroup::CycleIn_handler(NATIVE_INT_TYPE portNum, Svc::TimerVal& cycleStart) {
void ActiveRateGroup::CycleIn_handler(NATIVE_INT_TYPE portNum, Os::RawTime& cycleStart) {

Check notice

Code scanning / CodeQL

Use of basic integral type Note

portNum uses the basic integral type int rather than a typedef with size and signedness.
Svc/ActiveRateGroup/ActiveRateGroup.cpp
@@ -103,7 +108,7 @@

}

void ActiveRateGroup::CycleIn_preMsgHook(NATIVE_INT_TYPE portNum, Svc::TimerVal& cycleStart) {
void ActiveRateGroup::CycleIn_preMsgHook(NATIVE_INT_TYPE portNum, Os::RawTime& cycleStart) {

Check notice

Code scanning / CodeQL

Use of basic integral type Note

portNum uses the basic integral type int rather than a typedef with size and signedness.
Svc/PassiveRateGroup/PassiveRateGroup.cpp
@@ -38,8 +38,8 @@
}


void PassiveRateGroup::CycleIn_handler(NATIVE_INT_TYPE portNum, Svc::TimerVal& cycleStart) {
TimerVal end;
void PassiveRateGroup::CycleIn_handler(NATIVE_INT_TYPE portNum, Os::RawTime& cycleStart) {

Check notice

Code scanning / CodeQL

Use of basic integral type Note

portNum uses the basic integral type int rather than a typedef with size and signedness.
Svc/RateGroupDriver/RateGroupDriver.cpp
@@ -41,7 +41,7 @@

}

void RateGroupDriver::CycleIn_handler(NATIVE_INT_TYPE portNum, Svc::TimerVal& cycleStart) {
void RateGroupDriver::CycleIn_handler(NATIVE_INT_TYPE portNum, Os::RawTime& cycleStart) {

Check notice

Code scanning / CodeQL

Use of basic integral type Note

portNum uses the basic integral type int rather than a typedef with size and signedness.
This was referenced Oct 8, 2024
Merged
Merged
Merged
github-advanced-security[bot]
github-advanced-security bot found potential problems Oct 8, 2024
View reviewed changes
Fw/Time/TimeInterval.cpp
Comment on lines +80 to +81
TimeInterval::Comparison TimeInterval ::
compare(

Check notice

Code scanning / CodeQL

Long function without assertion Note

All functions of more than 10 lines should have at least one assertion.
Fw/Time/TimeInterval.cpp
Comment on lines +104 to +105
TimeInterval TimeInterval ::
add(

Check notice

Code scanning / CodeQL

Long function without assertion Note

All functions of more than 10 lines should have at least one assertion.
Fw/Time/TimeInterval.cpp Fixed Show fixed Hide fixed
Fw/Time/TimeInterval.cpp
}
}

#ifdef BUILD_UT

Check notice

Code scanning / CodeQL

Conditional compilation Note

Use of conditional compilation must be kept to a minimum.
Fw/Time/TimeInterval.hpp
SERIALIZED_SIZE = sizeof(U32) * 2
};

TimeInterval() = default; // !< Default constructor

Check notice

Code scanning / CodeQL

More than one statement per line Note

This line contains 2 statements; only one is allowed.
Os/RawTime.cpp
return this->m_delegate.deserialize(buffer);
}

RawTime::Status RawTime::getDiffUsec(const RawTime& other, U32& result) const {

Check notice

Code scanning / CodeQL

Long function without assertion Note

All functions of more than 10 lines should have at least one assertion.
Os/RawTime.hpp
};

//! \brief default constructor
RawTimeInterface() = default;

Check notice

Code scanning / CodeQL

More than one statement per line Note

This line contains 2 statements; only one is allowed.
Svc/ActiveRateGroup/ActiveRateGroup.cpp
@@ -52,12 +52,12 @@
this->log_DIAGNOSTIC_RateGroupStarted();
}

void ActiveRateGroup::CycleIn_handler(NATIVE_INT_TYPE portNum, Svc::TimerVal& cycleStart) {
void ActiveRateGroup::CycleIn_handler(NATIVE_INT_TYPE portNum, Os::RawTime& cycleStart) {

Check notice

Code scanning / CodeQL

Long function without assertion Note

All functions of more than 10 lines should have at least one assertion.
Svc/PassiveRateGroup/PassiveRateGroup.cpp
@@ -38,8 +38,8 @@
}


void PassiveRateGroup::CycleIn_handler(NATIVE_INT_TYPE portNum, Svc::TimerVal& cycleStart) {
TimerVal end;
void PassiveRateGroup::CycleIn_handler(NATIVE_INT_TYPE portNum, Os::RawTime& cycleStart) {

Check notice

Code scanning / CodeQL

Long function without assertion Note

All functions of more than 10 lines should have at least one assertion.
Svc/RateGroupDriver/RateGroupDriver.cpp
@@ -41,7 +41,7 @@

}

void RateGroupDriver::CycleIn_handler(NATIVE_INT_TYPE portNum, Svc::TimerVal& cycleStart) {
void RateGroupDriver::CycleIn_handler(NATIVE_INT_TYPE portNum, Os::RawTime& cycleStart) {

Check notice

Code scanning / CodeQL

Long function without assertion Note

All functions of more than 10 lines should have at least one assertion.
github-advanced-security[bot]
github-advanced-security bot found potential problems Oct 9, 2024
View reviewed changes
Os/test/ut/rawtime/RawTimeRules.cpp

Os::RawTime::Status status = zero_time.getDiffUsec(now_time, result);
ASSERT_EQ(status, Os::RawTime::Status::OP_OVERFLOW);
}

Check warning

Code scanning / CppCheck

Could not find a newline character at the end of the file. Warning test

Could not find a newline character at the end of the file.
thomas-bc
thomas-bc commented Oct 11, 2024
View reviewed changes
Drv/LinuxUartDriver/LinuxUartDriver.cpp Outdated Show resolved Hide resolved
Fw/Time/TimeInterval.cpp Outdated Show resolved Hide resolved
Os/CMakeLists.txt Outdated Show resolved Hide resolved
Os/IntervalTimer.cpp Outdated Show resolved Hide resolved
Os/Posix/CMakeLists.txt Outdated Show resolved Hide resolved
Os/Stub/test/ut/StubRawTimeTests.cpp Show resolved Hide resolved
Os/test/ut/IntervalTimerTest.cpp Outdated Show resolved Hide resolved
Svc/ActiveRateGroup/ActiveRateGroup.cpp Outdated Show resolved Hide resolved
Svc/PassiveRateGroup/PassiveRateGroup.cpp Outdated Show resolved Hide resolved
config/FpConfig.h Outdated Show resolved Hide resolved
thomas-bc and others added 6 commits October 11, 2024 15:48
@thomas-bc
Peer review changes
fe37434
@thomas-bc
Move Os.RawTime to Os/Types.fpp
73030f9
@thomas-bc
Fix unused variable
d338fe2
@thomas-bc
Fix spelling and comments
d12e97c
@thomas-bc
spelling extravaganza
36a728e
@thomas-bc
Update metadata
b890474 
check-spelling run (pull_request_target) for os-interval-timer

Signed-off-by: check-spelling-bot <check-spelling-bot@users.noreply.github.com>
on-behalf-of: @check-spelling <check-spelling-bot@check-spelling.dev>
github-advanced-security[bot]
github-advanced-security bot found potential problems Oct 12, 2024
View reviewed changes
Fw/Time/TimeInterval.cpp
Comment on lines +121 to +122
TimeInterval TimeInterval ::
sub(

Check notice

Code scanning / CodeQL

Long function without assertion Note

All functions of more than 10 lines should have at least one assertion.
Os/IntervalTimer.cpp

namespace Os {

IntervalTimer::IntervalTimer() : m_startTime(), m_stopTime() {}

Check notice

Code scanning / CodeQL

More than one statement per line Note

This line contains 2 statements; only one is allowed.
Os/Posix/RawTime.cpp
return Status::OP_OK;
}

PosixRawTime::Status PosixRawTime::getTimeInterval(const Os::RawTime& other, Fw::TimeInterval& interval) const {

Check notice

Code scanning / CodeQL

Long function without assertion Note

All functions of more than 10 lines should have at least one assertion.
Os/Posix/RawTime.cpp
return buffer.serialize(static_cast<U32>(this->m_handle.m_timespec.tv_nsec));
}

Fw::SerializeStatus PosixRawTime::deserialize(Fw::SerializeBufferBase& buffer) {

Check notice

Code scanning / CodeQL

Long function without assertion Note

All functions of more than 10 lines should have at least one assertion.
github-advanced-security[bot]
github-advanced-security bot found potential problems Oct 12, 2024
View reviewed changes
Os/RawTime.hpp
public:

// Serialization size for RawTime objects, configured in config/FpConfig.h
static const FwSizeType SERIALIZED_SIZE = FW_RAW_TIME_SERIALIZATION_MAX_SIZE;

Check notice

Code scanning / CodeQL

Use of basic integral type Note

SERIALIZED_SIZE uses the basic integral type unsigned long rather than a typedef with size and signedness.
@LeStarch LeStarch self-requested a review October 14, 2024 17:06
LeStarch
LeStarch approved these changes Oct 14, 2024
View reviewed changes
Copy link
Collaborator

@LeStarch LeStarch left a comment

Choose a reason for hiding this comment

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

Confirmed review fixes are in!

@LeStarch LeStarch merged commit 564cb57 into nasa:devel Oct 14, 2024
34 of 35 checks passed
@thomas-bc thomas-bc added the Update Instructions Needed Need to add instructions in the release notes for updates. label Oct 14, 2024
@thomas-bc
Copy link
Collaborator Author

Breaking change: delay() now takes a Fw::TimeInterval instead of Fw::Time

@thomas-bc thomas-bc deleted the os-interval-timer branch January 14, 2025 19:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@LeStarch LeStarch LeStarch approved these changes

Assignees
No one assigned
Labels
Update Instructions Needed Need to add instructions in the release notes for updates.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@thomas-bc @LeStarch