CAN Rearchitecture #337

cindyli-13 · 2021-03-27T22:08:15Z

The new CAN library is called CANInterface and manages 2 CANBuses. Our old CAN infrastructure is still supported by using CANBus the same way as we previously did instead of using CANInterface. Currently, apps/test-can has a simple test app showing how to use the new CAN library. Also see apps/test-can/include/CANConfig.h for setting up CAN message maps, this is how we will define all the CAN signals that a specific board will receive/transmit. One-shot messages aren't put in the message map and are instead processed by the CANMsgHandler.

Update:
CAN hw filter setup is currently broken for CAN bus 2 due to this Mbed bug. Currently waiting for the fix to get approved.

cindyli-13

Left some initial comments. The CANInterface lib is functionally complete but some things still need to be polished

cindyli-13 · 2021-03-28T19:20:28Z

apps/test-can/include/CANConfig.h

+     }},
+
+    // Msg 2
+};


For each target, we'll need to configure a RX msg map and a TX msg map that lists all the messages that target is receiving / transmitting (streamed messages only!). Msg map structure is:

{ msg1Name : {signal1Name : signal1Value, signal2Name : signal2Value, ...}, msg2Name : {signal1Name : signal1Value, signal2Name : signal2Value, ...}, ... }

cindyli-13 · 2021-03-28T19:21:36Z

apps/test-can/include/CANConfig.h

+
+const static CANMsg::CANMsgHandlerMap rxOneShotMsgHandler = {
+    {COMMON_TEST_MESSAGE1, &handle_test_msg_one_shot},
+};


the CANMsgHandlerMap is the same as how we used it before, but this is only for received one-shot messages

cindyli-13 · 2021-03-28T19:22:28Z

apps/test-can/src/main_old.cpp

@@ -0,0 +1,71 @@
+#include "CANBus.h"


This is the old test-can app, leaving it here for now if we even want to use the old CAN method

apps/test-can/src/main.cpp

cindyli-13 · 2021-03-28T19:24:21Z

lib/can/include/CANInterface.h

+  static constexpr osPriority RX_POSTMAN_THREAD_PRIORITY   = osPriorityRealtime;
+  static constexpr osPriority RX_CLIENT_THREAD_PRIORITY    = osPriorityAboveNormal;
+  static constexpr osPriority TX_PROCESSOR_THREAD_PRIORITY = osPriorityBelowNormal;
+  static constexpr std::chrono::milliseconds TX_INTERDELAY = 1ms;


Do we need this TX interdelay?

cindyli-13 · 2021-03-28T19:33:10Z

lib/can/src/CANInterface.cpp

+    // If message is one-shot, process message
+    if (m_rxOneShotMsgHandler->find(msg.getID()) != m_rxOneShotMsgHandler->end()) {
+      m_rxOneShotMsgHandler->at(msg.getID())(msg);
+    }


In the rxClient thread, received one-shot messages get processed immediately by calling the message handler function in the CANMsgHandler. Streamed messages get unpacked and put into the RX msg map.

cindyli-13 · 2021-03-28T19:34:30Z

lib/can/src/CANInterface.cpp

+        MBED_WARNING(MBED_MAKE_ERROR(MBED_MODULE_PLATFORM, MBED_ERROR_CODE_INVALID_DATA_DETECTED),
+                     "CAN RX message unpacking failed");


I made these all MBED_WARNINGs instead of MBED_ERRORs because message packing and mailbox full faults shouldn't crash the program

cindyli-13 · 2021-03-28T19:46:33Z

lib/can/src/CANInterface.cpp

+      }
+    }
+
+    ThisThread::sleep_until(startTime + TX_PERIOD);


Replaced ThisThread::sleep_for() with ThisThread::sleep_until() cuz the TX interdelay is already 1ms, so TX messages weren't actually getting sent every 10ms. Although, if there's more than 10 TX messages to send then the TX interdelay will still make the transmit period greater than 10ms.

cindyli-13 · 2021-03-28T19:49:54Z

lib/can/src/CANInterface.cpp

+bool CANInterface::sendOneShotMessage(CANMsg &msg) {
+  CANMsg *mail = m_txMailboxOneShot.try_alloc_for(1ms);
+  if (mail) {
+    *mail = msg;
+    MBED_ASSERT(m_txMailboxOneShot.put(mail) == osOK);
+    return true;
+  }
+  return false;
+}
+
+bool CANInterface::updateStreamedSignal(HWBRIDGE::CANID msgID, HWBRIDGE::CANSIGNAL signalName,
+                                        HWBRIDGE::CANSignalValue_t signalValue) {
+  return m_txStreamedMsgMap->setSignalValue(msgID, signalName, signalValue);
+}


TX one-shots are by message: the app will request the CAN lib to send out the entire one-shot message. These get queued into the m_txMailboxOneShot mailbox to get transmitted in the txProcessor thread.

TX streamed are by signal: the app will request the CAN lib to update the value of a TX signal. These are periodically transmitted in the txProcessor thread.

cindyli-13 · 2021-03-28T19:53:00Z

lib/can/src/CANInterface.cpp

+  return m_rxStreamedMsgMap->getSignalValue(msgID, signalName, signalValue);
+}
+
+void CANInterface::switchCANBus(HWBRIDGE::CANBUSID canBusID) {


This switch CAN bus feature isn't really robust rn, still a WIP

cindyli-13 · 2021-04-03T00:33:10Z

apps/arm/include/CANConfig.h

+static CANMsgMap rxStreamedMsgMap = {
+    {CANID::ARM_SET_JOINT_POSITION,
+     {
+         {CANSIGNAL::ARM_SET_TURNTABLE_POSITION, 0},


I still haven't fleshed out the SNA handling for scaled signals, so leaving these as 0 for now. Some SNA checks are also missing right now

cindyli-13 · 2021-04-03T00:34:51Z

apps/gimbal/src/main.cpp

-  printf("\r\n\r\n");
-  printf("GIMBAL APPLICATION STARTED\r\n");
-  printf("=======================\r\n");
+static mbed_error_status_t commonSwitchCANBus(CANMsg& msg) {


This switch CAN bus function is common to all the targets. It might be a good idea to have this inside the CAN library

cindyli-13 · 2021-04-03T00:35:50Z

apps/pdb/src/main.cpp

@@ -9,16 +9,15 @@ AnalogIn railBattery(RAIL_BATTERY_ANLG_IN), rail5V(RAIL_5V_ANLG_IN), rail17V(RAI
    rail24V(RAIL_24V_ANLG_IN);  // add voltage range (as percentage) to hw bridge also allocate can id for reporting if
                                // outside range

-CANBus can1(CAN1_RX, CAN1_TX, HWBRIDGE::ROVERCONFIG::ROVER_CANBUS_FREQUENCY);
+CANBus can1(CAN1_RX, CAN1_TX, HWBRIDGE::ROVER_CANBUS_FREQUENCY_HZ);


PDB is still using the old CAN lib, will update it to the new CAN lib in the PDB app PR

this is also global, when it should be local imo

jkleiber

Looks good overall, just wanted to note that it would be even better if the CanInterface was a local variable to main() and passed to the functions needed instead of being accessed all over the place. Mostly a design thing. Also requested some comments in the CanInterface because some of the functions had no comments, but maybe it's obvious to someone who is familiar with this firmware.

jkleiber · 2021-04-16T13:38:51Z

apps/gimbal/src/main.cpp

-      break;
-  }
-}
+CANInterface can(CANConfig::config);


Is there any reason this is global?

jkleiber · 2021-04-16T13:45:03Z

apps/science/src/main.cpp

+Sensor::AdafruitSTEMMA moistureSensor(TEMP_MOIST_I2C_SDA, TEMP_MOIST_I2C_SCL);

-// Felix TODO: Brose through logic and make sure there aren't any silly mistakes.
+CANInterface can(CANConfig::config);


I also recommend not making these global. Global variables may lead to unexpected behavior and it's better to pass the sensor and the can interface references to the functions where they are needed imo

jkleiber · 2021-04-16T13:47:17Z

apps/test-can/src/main.cpp

-DigitalOut ledRX(LED2);
-Timer timer;
-uint8_t counter = 0;
+CANInterface can(CANConfig::config);


Just marking this as global for attention, see other comments

jkleiber · 2021-04-16T13:53:31Z

lib/can/src/CANInterface.cpp

+  }
+}
+
+bool CANInterface::sendOneShotMessage(CANMsg &msg, Kernel::Clock::duration_u32 timeout) {


comment this

jkleiber · 2021-04-16T13:53:53Z

lib/can/src/CANInterface.cpp

+bool CANInterface::getRXSignalValue(HWBRIDGE::CANID msgID, HWBRIDGE::CANSIGNAL signalName,
+                                    HWBRIDGE::CANSignalValue_t &signalValue) {
+  m_rxMutex.lock();
+  bool success = (m_rxMsgMap != nullptr) && m_rxMsgMap->getSignalValue(msgID, signalName, signalValue);


comment this

jkleiber · 2021-04-16T13:54:03Z

lib/can/src/CANInterface.cpp

+bool CANInterface::setTXSignalValue(HWBRIDGE::CANID msgID, HWBRIDGE::CANSIGNAL signalName,
+                                    HWBRIDGE::CANSignalValue_t signalValue) {
+  m_txMutex.lock();
+  bool success = (m_txMsgMap != nullptr) && m_txMsgMap->setSignalValue(msgID, signalName, signalValue);


comment this

jkleiber · 2021-04-16T13:55:26Z

lib/can/src/CANInterface.cpp

+  return success;
+}
+
+bool CANInterface::setFilter(HWBRIDGE::CANFILTER filter, CANFormat format, uint16_t mask, int handle) {


Would be nice to comment this as well, idk what this does

jkleiber · 2021-04-16T13:56:20Z

apps/arm/src/main.cpp

-    MBED_ASSERT(mail_box.free(mail) == osOK);
-  }
-}
+CANInterface can(CANConfig::config);


global variables bad (see other comments)

jkleiber · 2021-04-16T13:57:03Z

apps/pdb/src/main.cpp

@@ -9,16 +9,15 @@ AnalogIn railBattery(RAIL_BATTERY_ANLG_IN), rail5V(RAIL_5V_ANLG_IN), rail17V(RAI
    rail24V(RAIL_24V_ANLG_IN);  // add voltage range (as percentage) to hw bridge also allocate can id for reporting if
                                // outside range

-CANBus can1(CAN1_RX, CAN1_TX, HWBRIDGE::ROVERCONFIG::ROVER_CANBUS_FREQUENCY);
+CANBus can1(CAN1_RX, CAN1_TX, HWBRIDGE::ROVER_CANBUS_FREQUENCY_HZ);


this is also global, when it should be local imo

cindyli-13 · 2021-05-08T21:12:09Z

apps/arm/src/main.cpp

+static inline float DEG_TO_RAD(float deg) {
+  return deg * M_PI / 180.0f;
 }


These DEG_TO_RAD and RAD_TO_DEG functions will be removed once the units library has been integrated by @upadhyaydhruv

…d gimbal

cindyli-13 · 2021-06-13T01:55:59Z

Gonna merge this PR so that we can continue development with the new CAN stuff

cindyli-13 added the CANBus label Mar 27, 2021

cindyli-13 mentioned this pull request Mar 27, 2021

CAN Rearchitecture uwrobotics/uwrt_mars_rover_hw_bridge#25

Merged

cindyli-13 commented Mar 28, 2021

View reviewed changes

cindyli-13 force-pushed the cindy/can-rearchitecture branch from 47ccf9e to cdd472a Compare April 3, 2021 00:31

cindyli-13 commented Apr 3, 2021

View reviewed changes

cindyli-13 marked this pull request as ready for review April 3, 2021 00:38

cindyli-13 requested review from younesr1 and wmmc88 April 3, 2021 00:40

jkleiber suggested changes Apr 16, 2021

View reviewed changes

cindyli-13 commented May 8, 2021

View reviewed changes

cindyli-13 added 20 commits May 25, 2021 22:01

CAN: got this tx signal mapping almost compiling

dea887a

CAN: More signal map work

ead983d

CAN: Got msg map working

7e9fdff

CAN: add mbed error warnings

5c21d31

CAN: add dual CAN bus support, add simple test app

dfc6d16

CAN: use std::unordered_map::contains()

baa29ae

CAN: add can filtering function

c31c512

CAN: start updating arm app

887b9bc

CAN: migrate apps to new CAN library

2c0203e

CAN: add switch CAN bus to apps, add PID tuning handler to science an…

482801c

…d gimbal

CAN: fix multiple filters, revert to try_get(), add can_load_test script

1b493da

CAN: add diagnostic messages

e09b757

CAN: add test-stress-can app

0e98564

CAN: edit CAN diagnostics messages

6aad401

CAN: fix clang format

8026072

CAN: make can load test script log streamed vs one-shots

7625abd

CAN: swap CAN 1 CAN 2 pins

d300f2b

CAN: Add RX one-shot messages to msg map

aed34f2

CAN: fix test-lookup-table

f8f4300

CAN: kinda fix hw filter issue (TODO: fix mbed bug)

2f8694c

cindyli-13 added 5 commits May 25, 2021 22:14

CAN: fix bus switching

740b758

CAN: minor fixes

81982fe

CAN: add safety check, fix signal value enum naming

24b8742

Add num CAN rx/tx faults signals to msg maps

550d512

CAN: rebase master

ad71c4d

cindyli-13 force-pushed the cindy/can-rearchitecture branch from e9c423a to ad71c4d Compare May 26, 2021 03:27

cindyli-13 added 4 commits May 25, 2021 23:29

remove leftover lib folder

63252e2

CAN: fix test-can build errors

251a6c5

Fix test-stress-can buld errors

958db4c

Update submodule commit

0070f7b

cindyli-13 merged commit 49f90e2 into master Jun 13, 2021

cindyli-13 deleted the cindy/can-rearchitecture branch June 13, 2021 01:56

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

CAN Rearchitecture #337

CAN Rearchitecture #337

cindyli-13 commented Mar 27, 2021 •

edited

Loading

cindyli-13 left a comment

cindyli-13 Mar 28, 2021

cindyli-13 Mar 28, 2021

cindyli-13 Mar 28, 2021 •

edited

Loading

cindyli-13 Mar 28, 2021

cindyli-13 Mar 28, 2021

cindyli-13 Mar 28, 2021

cindyli-13 Mar 28, 2021

cindyli-13 Mar 28, 2021 •

edited

Loading

cindyli-13 Mar 28, 2021

cindyli-13 Apr 3, 2021

cindyli-13 Apr 3, 2021

cindyli-13 Apr 3, 2021

jkleiber Apr 16, 2021

jkleiber left a comment

jkleiber Apr 16, 2021

jkleiber Apr 16, 2021

jkleiber Apr 16, 2021

jkleiber Apr 16, 2021

jkleiber Apr 16, 2021

jkleiber Apr 16, 2021

jkleiber Apr 16, 2021

jkleiber Apr 16, 2021

jkleiber Apr 16, 2021

cindyli-13 May 8, 2021

cindyli-13 commented Jun 13, 2021

		MBED_WARNING(MBED_MAKE_ERROR(MBED_MODULE_PLATFORM, MBED_ERROR_CODE_INVALID_DATA_DETECTED),
		"CAN RX message unpacking failed");

CAN Rearchitecture #337

CAN Rearchitecture #337

Conversation

cindyli-13 commented Mar 27, 2021 • edited Loading

cindyli-13 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cindyli-13 Mar 28, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cindyli-13 Mar 28, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jkleiber left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cindyli-13 commented Jun 13, 2021

cindyli-13 commented Mar 27, 2021 •

edited

Loading

cindyli-13 Mar 28, 2021 •

edited

Loading

cindyli-13 Mar 28, 2021 •

edited

Loading