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

Rx Queue #2

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
18 changes: 18 additions & 0 deletions Core/Inc/canardRxQueue.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
#ifndef CANARD_RX_QUEUE_H
#define CANARD_RX_QUEUE_H

#include <stdlib.h>
#include <canard.h>

struct CanardRxQueueItem {
struct CanardRxQueueItem* next;
CanardCANFrame frame;
};

Copy link
Contributor

Choose a reason for hiding this comment

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

What is the size of the queue? If there is no boundary for the size of the queue, then what if the rx processing speed is slower than the rx incoming speed? I guess the system would run out of the memory and hard fault.

I think it will be better to keep track of the length of the queue somehow.

extern struct CanardRxQueueItem* rxQueueHead;
extern struct CanardRxQueueItem* rxQueueTail;

void enqueueRxFrame(CanardCANFrame* rx_frame);
Copy link
Contributor

Choose a reason for hiding this comment

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

more comments to describe what the function does, its parameters, and return value are greatly appreciated.

struct CanardRxQueueItem* dequeueRxFrame();

#endif
47 changes: 47 additions & 0 deletions Core/Src/canardRxQueue.c
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
#include <stdio.h>
#include <stdlib.h>
#include <canard.h>
#include "canardRxQueue.h"

struct CanardRxQueueItem* rxQueueHead = NULL;
struct CanardRxQueueItem* rxQueueTail = NULL;

void enqueueRxFrame(CanardCANFrame* rx_frame) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Currently, this enqueue function returns nothing. It would be so much better if this function could return some sort of error code. For instance,
enum rxQueueErrorCode{ SUCCESS, EMPTYITEM, OVERFLOW }


// dynamically allocate memory
struct CanardRxQueueItem* new = (struct CanardRxQueueItem*)malloc(sizeof(struct CanardRxQueueItem));

if (new == NULL) {
printf("CanardRxQueueItem memory allocation failed");
return;
}

new->next = NULL;
new->frame = *rx_frame;

if (rxQueueTail) {
rxQueueTail->next = new;
} else {
rxQueueHead = new;
}

rxQueueTail = new;
}


struct CanardRxQueueItem* dequeueRxFrame() {

if (!rxQueueHead) {
return NULL; // queue is empty
}

struct CanardRxQueueItem* dequeuedItem = rxQueueHead;
rxQueueHead = rxQueueHead->next;

if (!rxQueueHead) {
rxQueueTail = NULL; // after dequeuing, the queue is empty
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Since you dynamically allocated the queue item, the allocated memory should be handled carefully when we are done with it. You should really explicitly write a comment somewhere to let people know they need to free the memory block themselves after they pop it from the queue. Or, more preferably, you handle it for the user.


return dequeuedItem;
}

17 changes: 15 additions & 2 deletions Core/Src/main.c
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@
#include "../dsdlc_generated/inc/dronecan_msgs.h"
#include <canard_stm32_driver.h>
#include "node_settings.h"
#include "canardRxQueue.h"

/* USER CODE END Includes */

Expand Down Expand Up @@ -149,17 +150,26 @@ void HAL_CAN_RxFifo0MsgPendingCallback(CAN_HandleTypeDef *hcan) {
// Receiving
CanardCANFrame rx_frame;

const uint64_t timestamp = HAL_GetTick() * 1000ULL;
const int16_t rx_res = canardSTM32Recieve(hcan, CAN_RX_FIFO0, &rx_frame);

if (rx_res < 0) {
printf("Receive error %d\n", rx_res);
}
else if (rx_res > 0) // Success - process the frame
{
canardHandleRxFrame(&canard, &rx_frame, timestamp);
enqueueRxFrame(&rx_frame);
}
}

void processCanardRxQueue() {
struct CanardRxQueueItem* currentRxFrame = dequeueRxFrame();
const uint64_t timestamp = HAL_GetTick() * 1000ULL;

canardHandleRxFrame(&canard, &currentRxFrame->frame, timestamp);

free(currentRxFrame);
}

// NOTE: All canard handlers and senders are based on this reference: https://dronecan.github.io/Specification/7._List_of_standard_data_types/
// Alternatively, you can look at the corresponding generated header file in the dsdlc_generated folder

Expand Down Expand Up @@ -594,6 +604,9 @@ int main(void)
/* USER CODE BEGIN 3 */

processCanardTxQueue(&hcan1);

processCanardRxQueue();

const uint64_t ts = HAL_GetTick();

if (ts >= next_1hz_service_at){
Expand Down