Longan-Labs / Arduino_CAN_BUS_MCP2515

Arduino CAN Bus library, MCP2515/MCP2551
https://www.longan-labs.cc/
MIT License
120 stars 371 forks source link

Setting Filters and Masks #5

Open hobbytronics opened 10 years ago

hobbytronics commented 10 years ago

I have been playing around with setting Masks and Filters. It took me a while to work out how to make it work correctly, but there is an omission in the code for setting the standard mask and filter. The 'standard' frame mask and filter can be used to filter on ID and also the first 2 data bytes. Unfortunately the code in mcp_can.cpp doesn't allow for filtering on the data bytes, only on the ID. Here is the current code.

void MCP_CAN::mcp2515_write_id( const INT8U mcp_addr, const INT8U ext, const INT32U id )
{
    uint16_t canid;
    INT8U tbufdata[4];

    canid = (uint16_t)(id & 0x0FFFF);

    if ( ext == 1)
    {
        tbufdata[MCP_EID0] = (INT8U) (canid & 0xFF);
        tbufdata[MCP_EID8] = (INT8U) (canid >> 8);
        canid = (uint16_t)(id >> 16);
        tbufdata[MCP_SIDL] = (INT8U) (canid & 0x03);
        tbufdata[MCP_SIDL] += (INT8U) ((canid & 0x1C) << 3);
        tbufdata[MCP_SIDL] |= MCP_TXB_EXIDE_M;
        tbufdata[MCP_SIDH] = (INT8U) (canid >> 5 );
    }
    else
    {
        tbufdata[MCP_SIDH] = (INT8U) (canid >> 3 );
        tbufdata[MCP_SIDL] = (INT8U) ((canid & 0x07 ) << 5);
        tbufdata[MCP_EID0] = 0;
        tbufdata[MCP_EID8] = 0;
    }
    mcp2515_setRegisterS( mcp_addr, tbufdata, 4 );
}

I have gotten around this by modifying the code and adding ext=2. Here is my modified code

void MCP_CAN::mcp2515_write_id( const INT8U mcp_addr, const INT8U ext, const INT32U id )
{
    uint16_t canid;
    INT8U tbufdata[4];

    canid = (uint16_t)(id & 0x0FFFF);

    if ( ext == 1) 
    {
        tbufdata[MCP_EID0] = (INT8U) (canid & 0xFF);
        tbufdata[MCP_EID8] = (INT8U) (canid >> 8);
        canid = (uint16_t)(id >> 16);
        tbufdata[MCP_SIDL] = (INT8U) (canid & 0x03);
        tbufdata[MCP_SIDL] += (INT8U) ((canid & 0x1C) << 3);
        tbufdata[MCP_SIDL] |= MCP_TXB_EXIDE_M;
        tbufdata[MCP_SIDH] = (INT8U) (canid >> 5 );
    }
    else if ( ext == 2) 
    {
        tbufdata[MCP_EID0] = (INT8U) (canid & 0xFF);
        tbufdata[MCP_EID8] = (INT8U) (canid >> 8);   
        canid = (uint16_t)(id >> 16);         
        tbufdata[MCP_SIDH] = (INT8U) (canid >> 3 );
        tbufdata[MCP_SIDL] = (INT8U) ((canid & 0x07 ) << 5);
    }
    else
    {
        tbufdata[MCP_SIDH] = (INT8U) (canid >> 3 );
        tbufdata[MCP_SIDL] = (INT8U) ((canid & 0x07 ) << 5);
        tbufdata[MCP_EID0] = 0;
        tbufdata[MCP_EID8] = 0;
    }
    mcp2515_setRegisterS( mcp_addr, tbufdata, 4 );
}

Can this fix be added to the code, or some similar modification be made?

coryjfowler commented 10 years ago

I was working on making an example sketch to demonstrate the mask and filtering abilities and stumbled upon this as well but I was not sure if anyone would really need to filter on the data bytes... Instead of calling the 'mcp2515_write_id' function, I was going to write a 'mcp2515_write_mf' function that would take advantage of the standard ID data bytes mask and filtering ability.

hobbytronics commented 10 years ago

Filtering on data bytes is probably not needed when obtaining data from a vehicle system, but the can bus protocol is being used for all manner of things from industrial automation to model train sets. One node could send out several different packets of information, distinguished by a 'packet type' byte. Hence the need for filtering on the data bytes.