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.

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

Os task refactor issue 2526 #2672

Merged
merged 29 commits into from
May 22, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
29 commits
Select commit Hold shift + click to select a range
b9b0ec1
Task refactor WIP -- compiles, but does not link
LeStarch Feb 14, 2024
e59c65b
Adding isCooperative method - WIP
LeStarch Apr 4, 2024
ac2df59
more WIP
LeStarch Apr 5, 2024
4a9d40a
Initial Os::Posix::Task implementaion
LeStarch Apr 9, 2024
ab06aac
Adding makeDelegate functions; Fixing existing UTs
LeStarch Apr 10, 2024
f3298ca
Touching up comments
LeStarch Apr 10, 2024
f0af2fc
Fixing Linux issues
LeStarch Apr 10, 2024
24a0e57
Adding type_traits import
LeStarch Apr 10, 2024
f2e9ded
Removing TaskId and reworking handle storage
LeStarch Apr 10, 2024
c957e16
Do not need to assert that a reference is not nullptr; compiler will …
kevin-f-ortega Apr 10, 2024
9926f7b
Starting UT development
LeStarch Apr 11, 2024
4c408a6
Start test works
LeStarch Apr 11, 2024
6732aa5
Working start and join rules; random test
LeStarch Apr 11, 2024
54314fa
Adding state, delay, and count tests
LeStarch Apr 12, 2024
05dacb9
Adding stub interface tests
LeStarch Apr 15, 2024
c5aaa12
sp
LeStarch Apr 15, 2024
c1e43e0
Registry tests
LeStarch Apr 22, 2024
49d84c8
Fixing UTs
LeStarch Apr 25, 2024
298fa51
Fixing PTHREAD_MIN_STACK for linux
LeStarch Apr 25, 2024
125d984
Missing newlines
LeStarch Apr 25, 2024
f6fd5dc
Fixing GPIO driver's task
LeStarch Apr 25, 2024
e6a9924
More build fixes
LeStarch Apr 26, 2024
8a273c6
Fixes for CI
LeStarch Apr 30, 2024
44908ed
Correcting errors in GNU only code
LeStarch May 1, 2024
ea40b98
Fixing review comments
LeStarch May 16, 2024
a7a02fe
Fixing sp
LeStarch May 16, 2024
63e02d5
Fixing RPI (again)
LeStarch May 16, 2024
9ffdee8
Removing divergent open rules from interface testing. Fixes #2733
LeStarch May 17, 2024
4e6645a
Fixing review comments
LeStarch May 22, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions .github/actions/spelling/expect.txt
Original file line number Diff line number Diff line change
Expand Up @@ -294,6 +294,7 @@ ect
edu
EGB
EHAs
eip
elist
ELOG
Elts
Expand Down Expand Up @@ -1012,6 +1013,7 @@ svipc
swcaegitadmin
synchronicity
synopsys
sysconf
SYSCONFDIR
SYSFS
sysinfo
Expand Down Expand Up @@ -1110,6 +1112,7 @@ toolset
topologyapp
Torvalds
totalram
tparam
treeview
trimwhitespace
trinomials
Expand Down
19 changes: 10 additions & 9 deletions Drv/Ip/SocketReadTask.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -23,17 +23,18 @@

SocketReadTask::~SocketReadTask() {}

void SocketReadTask::startSocketTask(const Fw::StringBase &name,
void SocketReadTask::start(const Fw::StringBase &name,

Check notice

Code scanning / CodeQL

Long function without assertion Note

All functions of more than 10 lines should have at least one assertion.
const bool reconnect,
const Os::Task::ParamType priority,
const Os::Task::ParamType stack,
const Os::Task::ParamType cpuAffinity) {
FW_ASSERT(not m_task.isStarted()); // It is a coding error to start this task multiple times
FW_ASSERT(m_task.getState() == Os::Task::State::NOT_STARTED); // It is a coding error to start this task multiple times
FW_ASSERT(not this->m_stop); // It is a coding error to stop the thread before it is started
m_reconnect = reconnect;
// Note: the first step is for the IP socket to open the port
Os::Task::TaskStatus stat = m_task.start(name, SocketReadTask::readTask, this, priority, stack, cpuAffinity);
FW_ASSERT(Os::Task::TASK_OK == stat, static_cast<NATIVE_INT_TYPE>(stat));
Os::Task::Arguments arguments(name, SocketReadTask::readTask, this, priority, stack, cpuAffinity);

Check warning

Code scanning / CodeQL

Unchecked function argument Warning

This use of parameter name has not been checked.

Check warning

Code scanning / CodeQL

Unchecked function argument Warning

This use of parameter priority has not been checked.

Check warning

Code scanning / CodeQL

Unchecked function argument Warning

This use of parameter stack has not been checked.

Check warning

Code scanning / CodeQL

Unchecked function argument Warning

This use of parameter cpuAffinity has not been checked.
Os::Task::Status stat = m_task.start(arguments);
FW_ASSERT(Os::Task::OP_OK == stat, static_cast<NATIVE_INT_TYPE>(stat));
}

SocketIpStatus SocketReadTask::startup() {
Expand All @@ -57,11 +58,11 @@
this->getSocketHandler().close();
}

Os::Task::TaskStatus SocketReadTask::joinSocketTask(void** value_ptr) {
return m_task.join(value_ptr);
Os::Task::Status SocketReadTask::join() {
return m_task.join();
}

void SocketReadTask::stopSocketTask() {
void SocketReadTask::stop() {
this->m_stop = true;
this->getSocketHandler().shutdown(); // Break out of any receives and fully shutdown
}
Expand All @@ -78,7 +79,7 @@
"[WARNING] Failed to open port with status %d and errno %d\n",
static_cast<POINTER_CAST>(status),
static_cast<POINTER_CAST>(errno));
(void) Os::Task::delay(SOCKET_RETRY_INTERVAL_MS);
(void) Os::Task::delay(SOCKET_RETRY_INTERVAL);
bocchino marked this conversation as resolved.
Show resolved Hide resolved
continue;
}

Expand All @@ -89,7 +90,7 @@
"[WARNING] Failed to open port with status %d and errno %d\n",
static_cast<POINTER_CAST>(status),
static_cast<POINTER_CAST>(errno));
(void) Os::Task::delay(SOCKET_RETRY_INTERVAL_MS);
(void) Os::Task::delay(SOCKET_RETRY_INTERVAL);
continue;
}

Expand Down
8 changes: 4 additions & 4 deletions Drv/Ip/SocketReadTask.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ class SocketReadTask {
* \param stack: stack size provided to the task. See: Os::Task::start. Default: TASK_DEFAULT, posix threads default
* \param cpuAffinity: cpu affinity provided to task. See: Os::Task::start. Default: TASK_DEFAULT, don't care
*/
void startSocketTask(const Fw::StringBase &name,
void start(const Fw::StringBase &name,
const bool reconnect = true,
const Os::Task::ParamType priority = Os::Task::TASK_DEFAULT,
const Os::Task::ParamType stack = Os::Task::TASK_DEFAULT,
Expand Down Expand Up @@ -106,17 +106,17 @@ class SocketReadTask {
* Called to stop the socket read task. It is an error to call this before the thread has been started using the
* startSocketTask call. This will stop the read task and close the client socket.
*/
void stopSocketTask();
void stop();

/**
* \brief joins to the stopping read task to wait for it to close
*
* Called to join with the read socket task. This will block and return after the task has been stopped with a call
* to the stopSocketTask method.
* \param value_ptr: a pointer to fill with data. Passed to the Os::Task::join call. NULL to ignore.
* \return: Os::Task::TaskStatus passed back from the Os::Task::join call.
* \return: Os::Task::Status passed back from the Os::Task::join call.
*/
Os::Task::TaskStatus joinSocketTask(void** value_ptr);
Os::Task::Status join();


PROTECTED:
Expand Down
10 changes: 8 additions & 2 deletions Drv/Ip/test/ut/SocketTestHelper.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
#include <unistd.h>
#include <cerrno>
#include <arpa/inet.h>
#include <IpCfg.hpp>

namespace Drv {
namespace Test {
Expand Down Expand Up @@ -65,7 +66,7 @@ bool wait_on_change(Drv::IpSocket &socket, bool open, U32 iterations) {
if (open == socket.isOpened()) {
return true;
}
Os::Task::delay(10);
Os::Task::delay(Fw::Time(0, 10000));
}
return false;
}
Expand All @@ -75,10 +76,15 @@ bool wait_on_started(Drv::IpSocket &socket, bool open, U32 iterations) {
if (open == socket.isStarted()) {
return true;
}
Os::Task::delay(10);
Os::Task::delay(Fw::Time(0, 10000));
}
return false;
}

U64 get_configured_delay_ms() {
return (static_cast<U64>(SOCKET_RETRY_INTERVAL.getSeconds()) * 1000) +
(static_cast<U64>(SOCKET_RETRY_INTERVAL.getUSeconds()) / 1000);
}

};
};
6 changes: 6 additions & 0 deletions Drv/Ip/test/ut/SocketTestHelper.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,12 @@ bool wait_on_change(Drv::IpSocket &socket, bool open, U32 iterations);
*/
bool wait_on_started(Drv::IpSocket &socket, bool open, U32 iterations);

/**
* Get the configured delay, converted to milliseconds
* @return SOCKET_RETRY_INTERVAL converted to milliseconds
*/
U64 get_configured_delay_ms();

};
};
#endif
9 changes: 5 additions & 4 deletions Drv/LinuxGpioDriver/LinuxGpioDriverComponentImpl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -396,13 +396,14 @@

}

Os::Task::TaskStatus LinuxGpioDriverComponentImpl ::
startIntTask(NATIVE_UINT_TYPE priority, NATIVE_UINT_TYPE stackSize, NATIVE_UINT_TYPE cpuAffinity) {
Os::Task::Status LinuxGpioDriverComponentImpl ::
startIntTask(Os::Task::ParamType priority, Os::Task::ParamType stackSize, Os::Task::ParamType cpuAffinity) {

Check notice

Code scanning / CodeQL

Use of basic integral type Note

priority uses the basic integral type unsigned long rather than a typedef with size and signedness.

Check notice

Code scanning / CodeQL

Use of basic integral type Note

stackSize uses the basic integral type unsigned long rather than a typedef with size and signedness.

Check notice

Code scanning / CodeQL

Use of basic integral type Note

cpuAffinity uses the basic integral type unsigned long rather than a typedef with size and signedness.
Os::TaskString name;
name.format("GPINT_%s",this->getObjName()); // The task name can only be 16 chars including null
Os::Task::TaskStatus stat = this->m_intTask.start(name, LinuxGpioDriverComponentImpl::intTaskEntry, this, priority, stackSize, cpuAffinity);
Os::Task::Arguments arguments(name, LinuxGpioDriverComponentImpl::intTaskEntry, this, priority, stackSize, cpuAffinity);

Check warning

Code scanning / CodeQL

Unchecked function argument Warning

This use of parameter priority has not been checked.

Check warning

Code scanning / CodeQL

Unchecked function argument Warning

This use of parameter stackSize has not been checked.

Check warning

Code scanning / CodeQL

Unchecked function argument Warning

This use of parameter cpuAffinity has not been checked.
Os::Task::Status stat = this->m_intTask.start(arguments);

if (stat != Os::Task::TASK_OK) {
if (stat != Os::Task::OP_OK) {
DEBUG_PRINT("Task start error: %d\n",stat);
}

Expand Down
6 changes: 3 additions & 3 deletions Drv/LinuxGpioDriver/LinuxGpioDriverComponentImpl.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -45,9 +45,9 @@ namespace Drv {
~LinuxGpioDriverComponentImpl();

//! Start interrupt task
Os::Task::TaskStatus startIntTask(NATIVE_UINT_TYPE priority = Os::Task::TASK_DEFAULT,
NATIVE_UINT_TYPE stackSize = Os::Task::TASK_DEFAULT,
NATIVE_UINT_TYPE cpuAffinity = Os::Task::TASK_DEFAULT);
Os::Task::Status startIntTask(Os::Task::ParamType priority = Os::Task::TASK_DEFAULT,
LeStarch marked this conversation as resolved.
Show resolved Hide resolved
Os::Task::ParamType stackSize = Os::Task::TASK_DEFAULT,
Os::Task::ParamType cpuAffinity = Os::Task::TASK_DEFAULT);

//! configure GPIO
enum GpioDirection {
Expand Down
6 changes: 3 additions & 3 deletions Drv/LinuxGpioDriver/LinuxGpioDriverComponentImplStub.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -43,9 +43,9 @@ namespace Drv {
return false;
}

Os::Task::TaskStatus LinuxGpioDriverComponentImpl ::
startIntTask(NATIVE_UINT_TYPE priority, NATIVE_UINT_TYPE stackSize, NATIVE_UINT_TYPE cpuAffinity) {
return Os::Task::TASK_OK;
Os::Task::Status LinuxGpioDriverComponentImpl ::
startIntTask(Os::Task::ParamType priority, Os::Task::ParamType stackSize, Os::Task::ParamType cpuAffinity) {
return Os::Task::OP_OK;
}

LinuxGpioDriverComponentImpl ::
Expand Down
16 changes: 7 additions & 9 deletions Drv/LinuxUartDriver/LinuxUartDriver.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -355,7 +355,7 @@
status = RecvStatus::RECV_ERROR;
comp->recv_out(0, buff, status);
// to avoid spinning, wait 50 ms
Os::Task::delay(50);
Os::Task::delay(Fw::Time(0, 50));

Check warning

Code scanning / CodeQL

Unchecked return value Warning

The return value of non-void function
delay
is not checked.
continue;
}

Expand Down Expand Up @@ -389,21 +389,19 @@
}
}

void LinuxUartDriver ::startReadThread(NATIVE_UINT_TYPE priority,
NATIVE_UINT_TYPE stackSize,
NATIVE_UINT_TYPE cpuAffinity) {
void LinuxUartDriver ::start(Os::Task::ParamType priority, Os::Task::ParamType stackSize, Os::Task::ParamType cpuAffinity) {

Check notice

Code scanning / CodeQL

Use of basic integral type Note

priority uses the basic integral type unsigned long rather than a typedef with size and signedness.

Check notice

Code scanning / CodeQL

Use of basic integral type Note

stackSize uses the basic integral type unsigned long rather than a typedef with size and signedness.

Check notice

Code scanning / CodeQL

Use of basic integral type Note

cpuAffinity uses the basic integral type unsigned long rather than a typedef with size and signedness.
Os::TaskString task("SerReader");
Os::Task::TaskStatus stat =
this->m_readTask.start(task, serialReadTaskEntry, this, priority, stackSize, cpuAffinity);
FW_ASSERT(stat == Os::Task::TASK_OK, stat);
Os::Task::Arguments arguments(task, serialReadTaskEntry, this, priority, stackSize, cpuAffinity);

Check warning

Code scanning / CodeQL

Unchecked function argument Warning

This use of parameter priority has not been checked.

Check warning

Code scanning / CodeQL

Unchecked function argument Warning

This use of parameter stackSize has not been checked.

Check warning

Code scanning / CodeQL

Unchecked function argument Warning

This use of parameter cpuAffinity has not been checked.
Os::Task::Status stat = this->m_readTask.start(arguments);
FW_ASSERT(stat == Os::Task::OP_OK, stat);
}

void LinuxUartDriver ::quitReadThread() {
this->m_quitReadThread = true;
}

Os::Task::TaskStatus LinuxUartDriver ::join(void** value_ptr) {
return m_readTask.join(value_ptr);
Os::Task::Status LinuxUartDriver ::join() {
return m_readTask.join();
}

} // end namespace Drv
8 changes: 4 additions & 4 deletions Drv/LinuxUartDriver/LinuxUartDriver.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -77,15 +77,15 @@ class LinuxUartDriver : public LinuxUartDriverComponentBase {
//! start the serial poll thread.
//! buffSize is the max receive buffer size
//!
void startReadThread(NATIVE_UINT_TYPE priority = Os::Task::TASK_DEFAULT,
NATIVE_UINT_TYPE stackSize = Os::Task::TASK_DEFAULT,
NATIVE_UINT_TYPE cpuAffinity = Os::Task::TASK_DEFAULT);
void start(Os::Task::ParamType priority = Os::Task::TASK_DEFAULT,
Os::Task::ParamType stackSize = Os::Task::TASK_DEFAULT,
Os::Task::ParamType cpuAffinity = Os::Task::TASK_DEFAULT);

//! Quit thread
void quitReadThread();

//! Join thread
Os::Task::TaskStatus join(void** value_ptr);
Os::Task::Status join();

//! Destroy object LinuxUartDriver
//!
Expand Down
8 changes: 4 additions & 4 deletions Drv/TcpClient/test/ut/TcpClientTester.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ void TcpClientTester ::test_with_loop(U32 iterations, bool recv_thread) {
// Start up a receive thread
if (recv_thread) {
Os::TaskString name("receiver thread");
this->component.startSocketTask(name, true, Os::Task::TASK_DEFAULT, Os::Task::TASK_DEFAULT);
this->component.start(name, true, Os::Task::TASK_DEFAULT, Os::Task::TASK_DEFAULT);
}

// Loop through a bunch of client disconnects
Expand All @@ -55,7 +55,7 @@ void TcpClientTester ::test_with_loop(U32 iterations, bool recv_thread) {
if (not recv_thread) {
status1 = this->component.open();
} else {
EXPECT_TRUE(Drv::Test::wait_on_change(this->component.getSocketHandler(), true, SOCKET_RETRY_INTERVAL_MS/10 + 1));
EXPECT_TRUE(Drv::Test::wait_on_change(this->component.getSocketHandler(), true, Drv::Test::get_configured_delay_ms()/10 + 1));
}
EXPECT_TRUE(this->component.getSocketHandler().isOpened());
status2 = server.open();
Expand Down Expand Up @@ -87,8 +87,8 @@ void TcpClientTester ::test_with_loop(U32 iterations, bool recv_thread) {
}
// Properly stop the client on the last iteration
if ((1 + i) == iterations && recv_thread) {
this->component.stopSocketTask();
this->component.joinSocketTask(nullptr);
this->component.stop();
this->component.join();
} else {
this->component.close();
}
Expand Down
10 changes: 5 additions & 5 deletions Drv/TcpServer/test/ut/TcpServerTester.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -37,8 +37,8 @@ void TcpServerTester ::test_with_loop(U32 iterations, bool recv_thread) {
// Start up a receive thread
if (recv_thread) {
Os::TaskString name("receiver thread");
this->component.startSocketTask(name, true, Os::Task::TASK_DEFAULT, Os::Task::TASK_DEFAULT);
EXPECT_TRUE(Drv::Test::wait_on_started(this->component.getSocketHandler(), true, SOCKET_RETRY_INTERVAL_MS/10 + 1));
this->component.start(name, true, Os::Task::TASK_DEFAULT, Os::Task::TASK_DEFAULT);
EXPECT_TRUE(Drv::Test::wait_on_started(this->component.getSocketHandler(), true, Drv::Test::get_configured_delay_ms()/10 + 1));
} else {
serverStat = this->component.startup();
ASSERT_EQ(serverStat, SOCK_SUCCESS)
Expand All @@ -59,7 +59,7 @@ void TcpServerTester ::test_with_loop(U32 iterations, bool recv_thread) {
if (not recv_thread) {
status1 = this->component.open();
} else {
EXPECT_TRUE(Drv::Test::wait_on_change(this->component.getSocketHandler(), true, SOCKET_RETRY_INTERVAL_MS/10 + 1));
EXPECT_TRUE(Drv::Test::wait_on_change(this->component.getSocketHandler(), true, Drv::Test::get_configured_delay_ms()/10 + 1));
}
EXPECT_TRUE(this->component.getSocketHandler().isOpened());

Expand Down Expand Up @@ -93,8 +93,8 @@ void TcpServerTester ::test_with_loop(U32 iterations, bool recv_thread) {
// Properly stop the client on the last iteration
if ((1 + i) == iterations && recv_thread) {
this->component.shutdown();
this->component.stopSocketTask();
this->component.joinSocketTask(nullptr);
this->component.stop();
this->component.join();
} else {
this->component.close();
}
Expand Down
8 changes: 4 additions & 4 deletions Drv/Udp/test/ut/UdpTester.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ void UdpTester::test_with_loop(U32 iterations, bool recv_thread) {
// Start up a receive thread
if (recv_thread) {
Os::TaskString name("receiver thread");
this->component.startSocketTask(name, true, Os::Task::TASK_DEFAULT, Os::Task::TASK_DEFAULT);
this->component.start(name, true, Os::Task::TASK_DEFAULT, Os::Task::TASK_DEFAULT);
}

// Loop through a bunch of client disconnects
Expand All @@ -69,7 +69,7 @@ void UdpTester::test_with_loop(U32 iterations, bool recv_thread) {
<< "Port2: " << port2;

} else {
EXPECT_TRUE(Drv::Test::wait_on_change(this->component.getSocketHandler(), true, SOCKET_RETRY_INTERVAL_MS/10 + 1));
EXPECT_TRUE(Drv::Test::wait_on_change(this->component.getSocketHandler(), true, Drv::Test::get_configured_delay_ms()/10 + 1));
}
EXPECT_TRUE(this->component.getSocketHandler().isOpened());

Expand Down Expand Up @@ -106,8 +106,8 @@ void UdpTester::test_with_loop(U32 iterations, bool recv_thread) {
}
// Properly stop the client on the last iteration
if ((1 + i) == iterations && recv_thread) {
this->component.stopSocketTask();
this->component.joinSocketTask(nullptr);
this->component.stop();
this->component.join();
} else {
this->component.close();
}
Expand Down
Loading
Loading