Various issues when re-using can0/can1

[rnd-ash]

Hi, firstly, thanks for this library!

Brief TLDR into whats happening here for this odd use case. Essentially, im writing a J2534 driver for the Macchina M2 (Essentially an arduino Due with extra hardware), and am using the due_can library for interfacing with the CAN bus on ODB2 port.

The specification states that the device should terminate and re-use CAN Interfaces, So I wrote a custom wrapper class around your can0/can1 interfaces, a simplified version of the wrapper below:

#include "can_handler.h"
#include "pc_comm.h"

// Try to init CAN interface on one of the 2 avaliable built in interfaces
canbus_handler::canbus_handler(uint8_t id, uint32_t baud) {
    char buf[30] = {0x00};
    sprintf(buf, "CAN Speed: %lu bps", baud);
    PCCOMM::logToSerial(buf);
    switch(id) {
        case 0:
            PCCOMM::logToSerial("Setting up Can0");
            this->actLED = CAN0_LED;
            this->useCan1 = false;
            PCCOMM::logToSerial("Done");
            this->getIface().begin(baud);
            this->getIface().watchFor();
            PCCOMM::logToSerial("Done1");
            break;
        case 1:
            PCCOMM::logToSerial("Setting up Can1");
            this->actLED = CAN1_LED;
            this->useCan1 = true;
            PCCOMM::logToSerial("Done");
            this->getIface().begin(baud);
            this->getIface().watchFor();
            PCCOMM::logToSerial("Done1");
            break;
        default: // No more free CAN Interfaces! Alert the host driver
            PCCOMM::logToSerial("ERROR SETTING UP CAN INVALID ID");
            return;
    }
}

// Couldn't find a way to store references to can0 or can1 - Macchina freezes when
// attempting to access references to them??
// So do it this way instead
CANRaw canbus_handler::getIface() {
    return this->useCan1 ? Can0 : Can1;
}

// Set a filter on one of the Rx mailboxes
void canbus_handler::setFilter(uint8_t filterID, uint32_t canid, uint32_t mask, bool isExtended) {
    char buf[40] = {0x00};
    sprintf(buf, "Setting Rx Filters - MASK: 0x%04X, Filter: 0x%04X", mask, canid);
    PCCOMM::logToSerial(buf);
    for (int i = 0; i < 7; i++) {
        this->getIface().setRXFilter(filterID, canid, mask, isExtended);
    }
}

// Transmits a frame on the bus
void canbus_handler::transmit(CAN_FRAME f) {
    digitalWrite(this->actLED, LOW);
    char buf[100] = {0x00};
    sprintf(buf, "CAN Sending CAN Frame. ID: 0x%04X - DLC: %d", f.id, f.length);
    PCCOMM::logToSerial(buf);
    if (!this->getIface().sendFrame(f)) {
        PCCOMM::logToSerial("Error sending CAN Frame!");
    }
}
 // Attempts to read an avaliable frame from one of the mailboxes
bool canbus_handler::read(CAN_FRAME f) {
    if (this->getIface().available() > 0) {
        PCCOMM::logToSerial("CAN Frames avaliable");
        digitalWrite(this->actLED, LOW);
        this->getIface().read(f);
        return true;
    }
    return false;
}

void canbus_handler::reset() {
    PCCOMM::logToSerial("Resetting CAN Interface");
    //TODO Clear Tx and Rx buffers here, and put CAN To sleep
}

// nullptr implies they are not used yet
extern canbus_handler *h0 = nullptr; // First avaliable interface  (Use can0)
extern canbus_handler *h1 = nullptr; // Second avaliable interface (Use can1)

Bare in mind that these classes get disposed quite reguarly and a new one is created whenever the API asks for it.

With that in mind, I've observed the following issues:

1. Attempting to reference can0/can1 as a reference will cause a hard lockup when calling any method on the can interfaces (hence now the use of the getIface function in my code)
2. After a re-use of my class, transmitting no longer works! - I have a arduino with an mcp2515 module hooked up to the macchina attempting to read incomming can frames
3. read() does not read anything for some reason?, again, this is only after a deletion and re-creation of my handler class.

Any help much appreciated with this! - You can find the full macchina code base here if it is of any use.

[collin80]

It's awesome that you're trying to do that! I wanted to get J2534 support onto Macchina hardware but I haven't even started doing so yet. So, it's really cool that you're already into the task.

Now, a couple of things: First, you absolutely should be able to create pointers to the Can0 and Can1 objects. I'm sure I've been doing this all along. So, something is weird if that is locking things up. Secondly, I really wouldn't follow the specification here. It seems like you should be able to leave both CAN ports setup and just keep using them - with modifications to the set up as requested. Trying to create and destroy objects at runtime on a little MCU is not a good idea. Generally memory management on these little things is GARBAGE. Allowing object allocation and destruction is likely to very rapidly fragment the memory and blow up. Also, the due_can library somewhat assumes that everything was initialized once and then used for the life of the program. I don't know what happens if you constantly reinitialize it with objects that go in and out of scope. The expectation is that everything is coded much more simply since the processor isn't too especially powerful and the runtime library isn't very large.

TL;DR: Try doing the bare minimum possible to make it all work and I think it will be more cooperative. If you can ignore parts of the specification and still get it to work then ignore them.

[rnd-ash]

Why thank you :)

Feel free to test my code out - having testers is useful as I only have one Mercedes car so I'm only testing in Xentry, and I know they break the API slightly...

Yes, I shall do the bare minimum today and work my way up....and aha report back

[rnd-ash]

Unfortunatly not great...

I managed to get a simple sketch to work perfectly with the arduino!, but implementing that same method in the massive sketch for j2534 causes a crash within your library for some reason...

Firstly, i re-built my can_handler class:

#include "can_handler.h"
#include "pc_comm.h"

// Try to init CAN interface on one of the 2 avaliable built in interfaces
canbus_handler::canbus_handler(CANRaw* can) {
    if (!can) {
        PCCOMM::logToSerial("CONSTRUCTOR - WTF Can is null!?");
        return;
    }
    this->can = can;
}

// Is this interface handler free to be claimed?
bool canbus_handler::isFree() {
    return !this->inUse;
}

// Set a filter on one of the Rx mailboxes
void canbus_handler::setFilter(uint32_t canid, uint32_t mask, bool isExtended) {
    char buf[40] = {0x00};
    sprintf(buf, "Setting Rx Filters - MASK: 0x%04X, Filter: 0x%04X", mask, canid);
    PCCOMM::logToSerial(buf);
    for (int i = 0; i < 7; i++) {
        this->can->setRXFilter(canid, mask, isExtended);
    }
}

// Transmits a frame on the bus
void canbus_handler::transmit(CAN_FRAME f) {
    if (!this->can) {
        PCCOMM::logToSerial("TRANSMIT - WTF Can is null!?");
        return;
    }
    digitalWrite(this->actLED, LOW);
    char buf[100] = {0x00};
    sprintf(buf, "CAN Sending CAN Frame. ID: 0x%04X - DLC: %d", f.id, f.length);
    PCCOMM::logToSerial(buf);
    if (!this->can->sendFrame(f)) {
        PCCOMM::logToSerial("Error sending CAN Frame!");
    }
}
 // Attempts to read an avaliable frame from one of the mailboxes
bool canbus_handler::read(CAN_FRAME f) {
    if (this->can->available() > 0) {
        PCCOMM::logToSerial("CAN Frames avaliable");
        digitalWrite(this->actLED, LOW);
        this->can->read(f);
        return true;
    }
    return false;
}

// Locks the interface - Stops another channel from using it
void canbus_handler::lock(uint32_t baud) {
    PCCOMM::logToSerial("Locking CAN Interface");
    if (!this->can) {
        PCCOMM::logToSerial("LOCK - WTF Can is null!?");
        return;
    }
    this->can->init(baud);
    PCCOMM::logToSerial("CAN enabled andbaud set!");
    this->inUse = true;
}

// Unlocks the interface - Marks it as being avaliable for a new channel
void canbus_handler::unlock() {
    PCCOMM::logToSerial("Unlocking CAN Interface");
    if (!this->can) {
        PCCOMM::logToSerial("UNLOCK - WTF Can is null!?");
        return;
    }
    this->can->disable();
    PCCOMM::logToSerial("CAN Disabled!");
    this->inUse = false;
    //TODO Clear Tx and Rx buffers here, and put CAN To sleep
}

// nullptr implies they are not used yet
extern canbus_handler ch0 = canbus_handler(&Can0); // First avaliable interface  (Use can0)
extern canbus_handler ch1 = canbus_handler(&Can1); // Second avaliable interface (Use can1)

As you can see, i built some sanity checks with logging in this so i know where it crashes...

However, when running this, the following is observed from the Application DLLs standpoint:

[10:41:53.955] [DEBUG] CAN_FILT - Adding filter with ID 1
[10:41:53.955] [INFO ] MACCHINA LOG - Setting up ISO15765 Handler
[10:41:53.956] [INFO ] DllExport - PassThruIOCTL called
[10:41:53.957] [INFO ] MACCHINA LOG - Locking CAN Interface
[10:41:53.965] [INFO ] DllExport - PassThruIOCTL called
[10:41:53.965] [INFO ] DllExport - PassThruWriteMsgs called
[10:41:53.966] [INFO ] CHAN_SEND - Sending 1 messages to channel 1

Note the logs marked with MACCHINA LOG - These come from the Macchine itself via the logging functions. I also have LED's in the loop function that should blink to indicate the macchina is still looping, however as soon as it tries to lock the CAN Handler, it freezes up.

// Locks the interface - Stops another channel from using it
void canbus_handler::lock(uint32_t baud) {
    PCCOMM::logToSerial("Locking CAN Interface");
    if (!this->can) {
        PCCOMM::logToSerial("LOCK - WTF Can is null!?");
        return;
    }
    this->can->init(baud);
    PCCOMM::logToSerial("CAN enabled and baud set!");
    this->inUse = true;
}

Since it said "locking can interface", and didn't say "Can is null", but didn't say "CAN enabled and baud set", that implies it crashed when trying to set the baud rate!

Any help appretiated

[rnd-ash]

I don't understand this! I fixed it by simply copying can_due.h and can_due.cpp to my own project directory!? - WTF...

I tried it again to be sure and same result...if can_due.cpp and can_due.h are in my project directory, it works perfectly, else it locks up!

[collin80]

Well, that is a weird result. The only thing that should change is where the code is placed in the memory space. But, why would changing the link location change how it runs?

[rnd-ash]

no clue, i can order myself a JTAG debugger and see whats going on. This is a interesting mystery...

The only thing i can think of is that some weird header that is Macchina specific is being replaced with the Arduino Due specific header when the library is a global one, which could cause a lockup - Or should it if they use the same CPU??

[rnd-ash]

I have now included the can_due files in my own macchina project on github....so i need to find a way to remove those without breaking the project