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

Add Open Port Request on Sending Side of IpSocket #2683

Open
wants to merge 2 commits into
base: devel
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 2 additions & 0 deletions Drv/ByteStreamDriverModel/ByteStreamDriverModel.fpp
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,8 @@ module Drv {
ref pollBuffer: Fw.Buffer
) -> PollStatus

port ByteStreamClose()

@ Signal indicating the driver is ready to send and received data
port ByteStreamReady()
}
27 changes: 25 additions & 2 deletions Drv/Ip/IpSocket.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
#include <FpConfig.hpp>
#include <Fw/Types/StringUtils.hpp>
#include <sys/time.h>
#include <Fw/Logger/Logger.hpp>

// This implementation has primarily implemented to isolate
// the socket interface from the F' Fw::Buffer class.
Expand Down Expand Up @@ -125,6 +126,14 @@
this->m_lock.unLock();
}

void IpSocket::lockSocketMutex(){
this->m_socket_lock.lock();
}

void IpSocket::unLockSocketMutex(){
this->m_socket_lock.unlock();
}

void IpSocket::shutdown() {
this->close();
this->m_lock.lock();
Expand Down Expand Up @@ -160,10 +169,24 @@
SocketIpStatus IpSocket::send(const U8* const data, const U32 size) {
U32 total = 0;
I32 sent = 0;
// Prevent transmission before connection, or after a disconnect
if (this->m_fd == -1) {
SocketIpStatus status;
bool disconnected = false;

Check notice

Code scanning / CodeQL

Use of basic integral type Note

disconnected uses the basic integral type bool rather than a typedef with size and signedness.

// Open a network connection if it has not already been open
// Lock mutex to avoid competing opens from other threads
this->lockSocketMutex();
if((not this->isOpened())) {
disconnected = (status = this->open()) != SOCK_SUCCESS;
}
this->unLockSocketMutex();

if (disconnected) {
Fw::Logger::logMsg("[WARNING] Failed to open port on send side with status %d and errno %d\n", status, errno);
this->close();
// Network level will decide what to do next
return SOCK_DISCONNECTED;
}

// Attempt to send out data and retry as necessary
for (U32 i = 0; (i < SOCKET_MAX_ITERATIONS) && (total < size); i++) {
// Send using my specific protocol
Expand Down
18 changes: 18 additions & 0 deletions Drv/Ip/IpSocket.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -154,6 +154,23 @@ class IpSocket {
*/
void close();

/**
* \brief locks the mutex protecting the opening of the IP socket
*
* Locks the mutex meant to keep the opening of an IP socket an atomic operation. Should be followed by a call to
* unLockSocketMutex when the operation is complete
*/
void lockSocketMutex();

/**
* \brief unlocks the mutex protecting the opening of the IP socket
*
* Unlocks the mutex locked by lockSocketMutex and should be called when the atomic operation of opening the IP socket
* is complete
*/
void unLockSocketMutex();


/**
* \brief shutdown the socket
*
Expand Down Expand Up @@ -201,6 +218,7 @@ class IpSocket {
virtual I32 recvProtocol( U8* const data, const U32 size) = 0;

Os::Mutex m_lock;
Os::Mutex m_socket_lock;
NATIVE_INT_TYPE m_fd;
U32 m_timeoutSeconds;
U32 m_timeoutMicroseconds;
Expand Down
15 changes: 15 additions & 0 deletions Drv/Ip/SocketReadTask.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,22 @@
FW_ASSERT(pointer);
SocketIpStatus status = SOCK_SUCCESS;
SocketReadTask* self = reinterpret_cast<SocketReadTask*>(pointer);
bool disconnected;

Check notice

Code scanning / CodeQL

Use of basic integral type Note

disconnected uses the basic integral type bool rather than a typedef with size and signedness.
do {
// Open a network connection if it has not already been open
disconnected = false;
// Lock mutex to avoid competing opens from other threads
self->getSocketHandler().lockSocketMutex();
Copy link
Collaborator

Choose a reason for hiding this comment

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

While this code block looks right, I believe it needs to be below (after start-up). I think a start-up needs to be added to to send too (following the same pattern).

Copy link
Collaborator

Choose a reason for hiding this comment

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

Line 98 of your file contains the original open that we should replace with this code

if((not self->getSocketHandler().isOpened()) and (not self->m_stop)) {
disconnected = (status = self->open()) != SOCK_SUCCESS;
}
self->getSocketHandler().unLockSocketMutex();
if (disconnected) {
// comment out so message is not spammed when no IP endpoint exists

Check failure on line 84 in Drv/Ip/SocketReadTask.cpp

View workflow job for this annotation

GitHub Actions / Spell checking

`spammed` is not a recognized word. (unrecognized-spelling)
// Fw::Logger::logMsg("[WARNING] Failed to open port on read side with status %d and errno %d\n", status, errno);
Os::Task::delay(SOCKET_RETRY_INTERVAL_MS);

Check warning

Code scanning / CodeQL

Unchecked return value Warning

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

// Open a network connection if it has not already been open
if ((not self->getSocketHandler().isStarted()) and (not self->m_stop) and
((status = self->startup()) != SOCK_SUCCESS)) {
Expand Down