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

fix handler list semaphore and constructor ordering #76

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

fix handler list semaphore and constructor ordering #76

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
41d0a2c
canard++: fix HandlerList subclass semaphores
tpwrules Feb 21, 2025
ba94067
canard++: fix handler list constructor/destructor ordering
tpwrules Feb 21, 2025
309ee28
canard++: un-inline handler list methods
tpwrules Feb 21, 2025
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
56 changes: 26 additions & 30 deletions canard/handler_list.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,14 +43,6 @@ class HandlerList {
/// @param _index Index of the handler list
HandlerList(CanardTransferType _transfer_type, uint16_t _msgid, uint64_t _signature, uint8_t _index) NOINLINE_FUNC :
index(_index) {
if (index >= CANARD_NUM_HANDLERS) {
return;
}
#ifdef WITH_SEMAPHORE
WITH_SEMAPHORE(sem[index]);
#endif
next = head[index];
head[index] = this;
msgid = _msgid;
signature = _signature;
transfer_type = _transfer_type;
Expand All @@ -59,25 +51,6 @@ class HandlerList {
/// @brief delete copy constructor and assignment operator
HandlerList(const HandlerList&) = delete;

// destructor, remove the entry from the singly-linked list
virtual ~HandlerList() NOINLINE_FUNC {
#ifdef WITH_SEMAPHORE
WITH_SEMAPHORE(sem[index]);
#endif
HandlerList* entry = head[index];
if (entry == this) {
head[index] = next;
return;
}
while (entry != nullptr) {
if (entry->next == this) {
entry->next = next;
return;
}
entry = entry->next;
}
}

/// @brief accept a message if it is handled by this handler list
/// @param index Index of the handler list
/// @param msgid ID of the message/service
Expand Down Expand Up @@ -123,14 +96,37 @@ class HandlerList {
virtual void handle_message(const CanardRxTransfer& transfer) = 0;

protected:
virtual ~HandlerList() {}
uint8_t index;
HandlerList* next;

private:
static HandlerList* head[CANARD_NUM_HANDLERS];
#ifdef WITH_SEMAPHORE
static Canard::Semaphore sem[CANARD_NUM_HANDLERS];
#endif

// add ourselves to the handler list. the caller must be holding the semaphore.
void link(void) NOINLINE_FUNC {
next = head[index];
head[index] = this;
}

// remove ourselves from the handler list. the caller must be holding the semaphore.
void unlink(void) NOINLINE_FUNC {
HandlerList* entry = head[index];
if (entry == this) {
head[index] = next;
return;
}
while (entry != nullptr) {
if (entry->next == this) {
entry->next = next;
return;
}
entry = entry->next;
}
}

private:
static HandlerList* head[CANARD_NUM_HANDLERS];
uint16_t msgid;
uint64_t signature;
CanardTransferType transfer_type;
Expand Down
16 changes: 12 additions & 4 deletions canard/service_client.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,20 +36,28 @@ class Client : public HandlerList, public Sender {
/// @brief Client constructor
/// @param _interface Interface object
/// @param _cb Callback object
Client(Interface &_interface, Callback<rsptype> &_cb) :
Client(Interface &_interface, Callback<rsptype> &_cb) NOINLINE_FUNC :
HandlerList(CanardTransferTypeResponse, rsptype::cxx_iface::ID, rsptype::cxx_iface::SIGNATURE, _interface.get_index()),
Sender(_interface),
server_node_id(255),
cb(_cb) {
#ifdef WITH_SEMAPHORE
WITH_SEMAPHORE(sem[index]);
#endif
next = branch_head[index];
branch_head[index] = this;
link(); // link ourselves into the handler list now that we're in the branch list
}

// delete copy constructor and assignment operator
Client(const Client&) = delete;

// destructor, remove the entry from the singly-linked list
~Client() {
~Client() NOINLINE_FUNC {
#ifdef WITH_SEMAPHORE
WITH_SEMAPHORE(sem[index]);
#endif
unlink(); // unlink ourselves from the handler list before the branch list
Client<rsptype>* entry = branch_head[index];
if (entry == this) {
branch_head[index] = next;
Expand All @@ -66,7 +74,7 @@ class Client : public HandlerList, public Sender {

/// @brief handles incoming messages
/// @param transfer transfer object of the request
void handle_message(const CanardRxTransfer& transfer) override {
void handle_message(const CanardRxTransfer& transfer) override NOINLINE_FUNC {
rsptype msg {};
if (rsptype::cxx_iface::rsp_decode(&transfer, &msg)) {
// invalid decode
Expand Down Expand Up @@ -98,7 +106,7 @@ class Client : public HandlerList, public Sender {
/// @param msg message containing the request
/// @param canfd true if CAN FD is to be used
/// @return true if the request was put into the queue successfully
bool request(uint8_t destination_node_id, typename rsptype::cxx_iface::reqtype& msg, bool canfd) {
bool request(uint8_t destination_node_id, typename rsptype::cxx_iface::reqtype& msg, bool canfd) NOINLINE_FUNC {
#if !CANARD_ENABLE_CANFD
if (canfd) {
return false;
Expand Down
18 changes: 14 additions & 4 deletions canard/service_server.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,19 +37,29 @@ class Server : public HandlerList {
/// @param _interface Interface object
/// @param _cb Callback object
/// @param _index HandlerList instance id
Server(Interface &_interface, Callback<reqtype> &_cb) :
Server(Interface &_interface, Callback<reqtype> &_cb) NOINLINE_FUNC :
HandlerList(CanardTransferTypeRequest, reqtype::cxx_iface::ID, reqtype::cxx_iface::SIGNATURE, _interface.get_index()),
interface(_interface),
cb(_cb) {
// multiple servers are not allowed, so no list
#ifdef WITH_SEMAPHORE
WITH_SEMAPHORE(sem[index]);
#endif
link(); // link ourselves into the handler list
}

// delete copy constructor and assignment operator
Server(const Server&) = delete;

~Server() NOINLINE_FUNC {
#ifdef WITH_SEMAPHORE
WITH_SEMAPHORE(sem[index]);
#endif
unlink(); // unlink ourselves from the handler list
}

/// @brief handles incoming messages
/// @param transfer transfer object of the request
void handle_message(const CanardRxTransfer& transfer) override {
void handle_message(const CanardRxTransfer& transfer) override NOINLINE_FUNC {
reqtype msg {};
if (reqtype::cxx_iface::req_decode(&transfer, &msg)) {
// invalid decode
Expand All @@ -64,7 +74,7 @@ class Server : public HandlerList {
/// @param transfer transfer object of the request
/// @param msg message containing the response
/// @return true if the response was put into the queue successfully
bool respond(const CanardRxTransfer& transfer, typename reqtype::cxx_iface::rsptype& msg) {
bool respond(const CanardRxTransfer& transfer, typename reqtype::cxx_iface::rsptype& msg) NOINLINE_FUNC {
// encode the message
uint32_t len = reqtype::cxx_iface::rsp_encode(&msg, rsp_buf
#if CANARD_ENABLE_CANFD
Expand Down
14 changes: 8 additions & 6 deletions canard/subscriber.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,21 +38,26 @@ class Subscriber : public HandlerList {
/// @brief Subscriber Constructor
/// @param _cb callback function
/// @param _index HandlerList instance id
Subscriber(Callback<msgtype> &_cb, uint8_t _index) :
Subscriber(Callback<msgtype> &_cb, uint8_t _index) NOINLINE_FUNC :
HandlerList(CanardTransferTypeBroadcast, msgtype::cxx_iface::ID, msgtype::cxx_iface::SIGNATURE, _index),
cb (_cb) {
#ifdef WITH_SEMAPHORE
WITH_SEMAPHORE(sem[index]);
#endif
next = branch_head[index];
branch_head[index] = this;
link(); // link ourselves into the handler list now that we're in the branch list
}

// delete copy constructor and assignment operator
Subscriber(const Subscriber&) = delete;

// destructor, remove the entry from the singly-linked list
~Subscriber() {
~Subscriber() NOINLINE_FUNC {
#ifdef WITH_SEMAPHORE
WITH_SEMAPHORE(sem[index]);
#endif
unlink(); // unlink ourselves from the handler list before the branch list
Subscriber<msgtype>* entry = branch_head[index];
if (entry == this) {
branch_head[index] = next;
Expand All @@ -69,7 +74,7 @@ class Subscriber : public HandlerList {

/// @brief parse the message and call the callback
/// @param transfer transfer object
void handle_message(const CanardRxTransfer& transfer) override {
void handle_message(const CanardRxTransfer& transfer) override NOINLINE_FUNC {
msgtype msg {};
if (msgtype::cxx_iface::decode(&transfer, &msg)) {
// invalid decode
Expand All @@ -86,9 +91,6 @@ class Subscriber : public HandlerList {
private:
Subscriber<msgtype>* next;
static Subscriber<msgtype> *branch_head[CANARD_NUM_HANDLERS];
#ifdef WITH_SEMAPHORE
Canard::Semaphore sem[CANARD_NUM_HANDLERS];
#endif
Callback<msgtype> &cb;
};

Expand Down