Better parameter handling (#6)

* Add rx event handlers

* Fix compile errors and add constness

* Pass sender id

* Change api interface

* Revert changes

* Add filter fifo

* Add default setup

* Add receive function

* Remove interrupt activation

* Activate RX interrupts

* Merge remote changes

* Add event list constructor

* Fix compilation bug

* Send and receive multiple frames

* Refactor

* Refactor

* Fix compile error

* Use known syntax

* Refactor

* Provide storage

* Add frame end

* Add some documentation

* Turn macros into functions

* Refactor

* Refactor

* Fix compile error

* Use ExtendedIds

* Fix reception bug (fixes #4)

* Add packet headers

* Refactor

Co-authored-by: Riccardo998 <riccardo.storchi98@gmail.com>

Inc/mmr_can.h

@@ -4,6 +4,7 @@
 #include <stdint.h>
 #include <stdbool.h>
 #include "mmr_can_includes.h"
+#include "mmr_can_header.h"
 #include "mmr_can_types.h"
 #include "mmr_can_optimize.h"
 #include "mmr_can_binary_literals.h"
@@ -65,31 +66,13 @@ typedef struct {
 
 
 typedef struct {
-  CanId remoteId;
+  MmrCanHeader header;
   CanMailbox *mailbox;
   uint8_t *data;
   uint8_t length;
 } MmrCanPacket;
 
 
-/**
- * @brief
- * These values can be appended to the extended-id
- * portion of the CAN bus message (that is, the lower 5 bits
- * of the standard id)
- * 
- * They are used to check if a message is either standalone
- * or split into multiple frames
- * 
- * Multi-frame messages have a higher priority over normal ones
- */
-typedef enum {
-  MMR_CAN_MESSAGE_TYPE_MULTI_FRAME = B_(0010),
-  MMR_CAN_MESSAGE_TYPE_MULTI_FRAME_END = B_(0011),
-  MMR_CAN_MESSAGE_TYPE_NORMAL = B_(1000),
-} MmrCanMessageType;
-
-
 /**
  * @brief
  * Represents a CAN message
@@ -112,8 +95,4 @@ MmrCanFilterSettings MMR_CAN_GetDefaultFilterSettings();
 HalStatus MMR_CAN_Send(CanHandle *hcan, MmrCanPacket packet);
 HalStatus MMR_CAN_Receive(CanHandle *hcan, MmrCanMessage *result);
 
-
-bool MMR_CAN_IsMultiFrame(CanRxHeader *header);
-bool MMR_CAN_IsMultiFrameEnd(CanRxHeader *header);
-
 #endif /* INC_MMR_CAN_H_ */

Inc/mmr_can_header.h

@@ -0,0 +1,42 @@
+#ifndef INC_MMR_CAN_HEADER_H_
+#define INC_MMR_CAN_HEADER_H_
+
+#include <stdbool.h>
+#include "mmr_can_util.h"
+#include "mmr_can_binary_literals.h"
+
+/**
+ * @brief
+ * These values can be appended to the extended-id
+ * portion of the CAN bus message (that is, the lower 5 bits
+ * of the standard id)
+ * 
+ * They are used to check if a message is either standalone
+ * or split into multiple frames
+ * 
+ * When the priority and id fields are the same, multi-frame
+ * messages have a higher priority over normal ones
+ */
+typedef enum {
+  MMR_CAN_MESSAGE_TYPE_MULTI_FRAME = B_(0010),
+  MMR_CAN_MESSAGE_TYPE_MULTI_FRAME_END = B_(0011),
+  MMR_CAN_MESSAGE_TYPE_NORMAL = B_(1000),
+} MmrCanMessageType;
+
+
+/**
+ * @brief
+ * This struct encodes the values stored inside the
+ * extended id field of a CAN packet.
+ */
+typedef struct {
+  uint32_t priority : 5;
+  uint32_t senderId : 23;
+  MmrCanMessageType messageType : 4;
+} MmrCanHeader;
+
+
+bool MMR_CAN_IsMultiFrame(MmrCanHeader *header);
+bool MMR_CAN_IsMultiFrameEnd(MmrCanHeader *header);
+
+#endif /* INC_MMR_CAN_HEADER_H_ */

Inc/mmr_can_util.h

@@ -15,6 +15,27 @@
 
 #define min(a, b) ((a) < (b) ? a : b);
 #define mask(value, bits) (value & bits)
+#define convertTo(resultType, lvalue) (*interpretAs(resultType*, &(lvalue)))
+#define interpretAs(resultType, lvalue) ((resultType)(lvalue))
+
+
+/**
+ * @brief
+ * Represents the result of an asynchronous computation
+ *
+ * Either
+ *  - Error: the computation resulted in error
+ *  - Pending: the computation is still undergoing
+ *  - Completed: the computation has completed succesfully
+ *               and its results can be read
+ *
+ * Asynchronous logig can be easily implemented using State Machines
+ */
+typedef enum {
+  MMR_ASYNC_RESULT_ERROR,
+  MMR_ASYNC_RESULT_PENDING,
+  MMR_ASYNC_RESULT_COMPLETED,
+} MmrAsyncResult;
 
 
 #endif /* INC_MMR_CAN_UTIL_H_ */

Src/mmr_can.c

@@ -1,9 +1,13 @@
 #include "mmr_can.h"
 #include "mmr_can_util.h"
 
-static uint8_t maskIdLower5Bits(CanRxHeader *header);
-
 
+/**
+ * @brief
+ * Initializes the filter using the values
+ * from MMR_CAN_GetDefaultFilterSettings, and then
+ * starts the CAN
+ */
 HalStatus MMR_CAN_BasicSetupAndStart(CanHandle *hcan) {
   return
     MMR_CAN_FilterConfigDefault(hcan) |
@@ -31,12 +35,6 @@ HalStatus MMR_CAN_FilterConfig(CanHandle *hcan, MmrCanFilterSettings settings) {
 }
 
 
-CanFilterMask MMR_CAN_AlignStandardMask(CanFilterMask baseMask) {
-  static const uint8_t extendedMaskSurplusBytes = 5;
-  return baseMask << extendedMaskSurplusBytes;
-}
-
-
 MmrCanFilterSettings MMR_CAN_GetDefaultFilterSettings() {
   return (MmrCanFilterSettings) {
     .enabled = true,
@@ -46,16 +44,3 @@ MmrCanFilterSettings MMR_CAN_GetDefaultFilterSettings() {
     .slaveBankStart = 14,
   };
 }
-
-
-bool MMR_CAN_IsMultiFrame(CanRxHeader *header) {
-  return maskIdLower5Bits(header) == MMR_CAN_MESSAGE_TYPE_MULTI_FRAME;
-}
-
-bool MMR_CAN_IsMultiFrameEnd(CanRxHeader *header) {
-  return maskIdLower5Bits(header) == MMR_CAN_MESSAGE_TYPE_MULTI_FRAME_END;
-}
-
-static always_inline uint8_t maskIdLower5Bits(CanRxHeader *header) {
-  return mask(header->ExtId, B8_(0001, 1111));
-}

Src/mmr_can_events.c

@@ -10,12 +10,24 @@ static void __invokeAll(const MmrCanEventList *events, const MmrCanMessage *even
 static void __maybeInvoke(const MmrCanEventHandler handler, const MmrCanMessage *event);
 
 
+/**
+ * @brief
+ * Activates the CAN rx interrupts
+ *
+ * When one is fired, the callbacks provided inside the
+ * MmrCanEventList will be invoked
+ */
 HalStatus MMR_CAN_InitRxHandlers(CanHandle *hcan, const MmrCanEventList *rxEvents) {
   _rxEvents = rxEvents;
   return HAL_CAN_ActivateNotification(hcan, MMR_CAN_RX_INTERRUPT);
 }
 
 
+/**
+ * @brief
+ * Stores the message inside a byte buffer,
+ * and then sends it to every registered event handler
+ */
 static void __handleCanRxInterrupt(CanHandle *hcan) {
   static CanRxBuffer buffer = {};
   static MmrCanMessage event = {

Src/mmr_can_header.c

@@ -0,0 +1,11 @@
+#include "mmr_can_header.h"
+#include "mmr_can_optimize.h"
+
+
+bool MMR_CAN_IsMultiFrame(MmrCanHeader *header) {
+  return header->messageType == MMR_CAN_MESSAGE_TYPE_MULTI_FRAME;
+}
+
+bool MMR_CAN_IsMultiFrameEnd(MmrCanHeader *header) {
+  return header->messageType == MMR_CAN_MESSAGE_TYPE_MULTI_FRAME_END;
+}

Src/mmr_can_receive.c

@@ -1,54 +1,78 @@
 #include <stdbool.h>
 #include "mmr_can.h"
 
-static HalStatus receiveOne(CanHandle *hcan, CanRxHeader *header, uint8_t *result);
+static HalStatus receiveOne(ReceptionParams *rp);
-static HalStatus receiveAll(CanHandle *hcan, CanRxHeader *header, uint8_t *result);
+static HalStatus receiveAll(ReceptionParams *rp);
-static bool headerIsMultiFrame(CanRxHeader *header, CanId targetId);
+static bool headerIsMultiFrame(MmrCanHeader *header, CanId targetId);
 
 
+typedef struct {
+  CanHandle *handle;
+  uint8_t *result;
+  struct {
+    CanRxHeader rx;
+    MmrCanHeader mmr;
+  } headers;
+
+  uint8_t fifo;
+} ReceptionParams;
+
+
+/**
+ * @brief
+ * Reads a CAN message and stores it inside the
+ * given MmrCanMessage struct.
+ *
+ * If a multi-frame message is received, this function will block
+ * and read every frame for that particular message.
+ */
 HalStatus MMR_CAN_Receive(CanHandle *hcan, MmrCanMessage *result) {
-  CanRxHeader header = {};
+  ReceptionParams rp = {
-  uint8_t *dest = result->store;
+    .handle = hcan,
-  HalStatus status = receiveOne(hcan, &header, dest);
+    .result = interpretAs(uint8_t*, result->store),
+    .fifo = MMR_CAN_RX_FIFO,
+  };
 
-  result->senderId = header.ExtId;
+  HalStatus status = receiveOne(&rp);
-  if (MMR_CAN_IsMultiFrame(&header)) {
+  if (status != HAL_OK) {
-    status |= receiveAll(hcan, &header, dest);
+    return status;
+  }
+
+  result->senderId = rp.headers.mmr.senderId;
+  if (MMR_CAN_IsMultiFrame(&rp.headers.mmr)) {
+    status |= receiveAll(&rp);
   }
 
   return status;
 }
 
 
-static HalStatus receiveOne(
+static HalStatus receiveAll(ReceptionParams *rp) {
-  CanHandle *hcan,
+  CanId targetId = rp->headers.mmr.senderId;
-  CanRxHeader *header,
-  uint8_t *result
-) {
-  return HAL_CAN_GetRxMessage(hcan, MMR_CAN_RX_FIFO, header, result);
-}
-
-static HalStatus receiveAll(
-  CanHandle *hcan,
-  CanRxHeader *header,
-  uint8_t *result
-) {
-  CanId targetId = header->ExtId;
   HalStatus status = HAL_OK;
   do {
-    result += MMR_CAN_MAX_DATA_LENGTH;
+    rp->result += MMR_CAN_MAX_DATA_LENGTH;
-    status |= receiveOne(hcan, header, result);
+    status |= receiveOne(rp);
   } while (
-    headerIsMultiFrame(header, targetId) && status == HAL_OK
+    headerIsMultiFrame(&rp->headers.mmr, targetId) && status == HAL_OK
   );
 
   return status;
 }
 
 
-static bool headerIsMultiFrame(CanRxHeader *header, CanId targetId) {
+static HalStatus receiveOne(ReceptionParams *rp) {
+  HalStatus status =
+    HAL_CAN_GetRxMessage(rp->hcan, rp->fifo, &(rp->headers.rx), rp->result);
+
+  rp->headers.mmr = convertTo(MmrCanHeader, header->ExtId);
+  return status;
+}
+
+
+static always_inline bool headerIsMultiFrame(MmrCanHeader *header, CanId targetId) {
   return
     MMR_CAN_IsMultiFrame(header) &&
     !MMR_CAN_IsMultiFrameEnd(header) &&
-    header->DLC >= MMR_CAN_MAX_DATA_LENGTH;
+    header->senderId == targetId;
 }

Src/mmr_can_send.c

@@ -15,7 +15,7 @@ HalStatus MMR_CAN_Send(CanHandle *hcan, MmrCanPacket packet) {
     .IDE = CAN_ID_EXT,
     .RTR = CAN_RTR_DATA,
     .DLC = packet.length,
-    .ExtId = packet.remoteId << 5,
+    .ExtId = convertTo(uint32_t, packet.header),
     .TransmitGlobalTime = DISABLE,
   };