TWAI dual filters doesn't seem to work with 29-bit IDs

[kevinevans]

I'm trying out using the dual filters on TWAI with extended (29 bit) IDs, but I'm having some trouble getting it to actually filter. The docs state using dual filters on 29-bit IDs will only compare the upper 16 bits of the 29-bit identifier.


Here are the filters that I'm using:

Code: Select all

    uint16_t id1   = 0b0000000000000111; // lower 3 bits are 0b111!
    uint16_t mask1 = 0b0000000010000111;
    
    /**
     * For PDU2s, we just require PF=1111_xxxx (PGNs > 240)
     * id2   = 000 0 0 1111 0000 000
     * mask2 = 000 0 0 1111 0000 000
     */

    uint16_t id2   = 0b0000011110000000;
    uint16_t mask2 = 0b0000011110000000;

    // disable filter 2 for now
    id2=0xFFFF;
    mask2=0xFFFF;

    twai_mask_filter_config_t dual_config = twai_make_dual_filter(
        id1, mask1,
        id2, mask2,
        true
    );

    ESP_RETURN_ON_ERROR(
        twai_node_config_mask_filter(*handle, 0, &dual_config),
        TAG,
        "failed to config can id filters"
    );
    
    // twai_node_enable called after this part

I've disabled the second filter so I can test the first. When I use CAN ID 0x18E8E623 that works as expected and I can receive the message.

But when I use CAN ID 0x18E80123, this should NOT pass BUT it still gets received. The upper 16 bits of the 29-bit identifier are: 1 1000 1110 1000 000. The lower 3 bits are 000. Since mask1 is 111 and id1 is 111, the lower 3 bits must match to be received.

Here's what I mean:

Code: Select all

CAN ID 0x18E80123:

0001 1000 1110 1000
0000 0001 0010 0011

To 29 bits (29=32-3):
   1 1000 1110 1000
0000 0001 0010 0011

Upper 16 bits:
   1 1000 1110 1000
000
^^^ Lower 3 bits are 000!

The lower 3 bits 000 does not equal 111. Are the docs wrong here? Is it comparing the 32-bit identifier instead of the 29 bit ID? Is there something that I'm misunderstanding? Thank you!

Here is a short test program to see what I mean and to test out values:

Code: Select all

#include <stdint.h>
#include <stdio.h>


int main() {

    uint8_t addr = 0x66;
    // id1   = 000 0 0 0000 0000 <source upper 3 b>
    // mask1 = 000 0 0 0001 0000 <source & 0xFF upper 3b>
    
    uint16_t id1   = 0b0000000000000111;
    uint16_t mask1 = 0b0000000010000111; // | upper;

    uint32_t can_id = 0x18E8E623; // this should pass

    // can_id = 0x18E80123; // this should NOT pass
    
    // take upper 16 bits of the 29-bit identifier
    can_id = (can_id >> 13) & 0xFFFF;

    printf("can_id=%x, masked=%x, id1=%x", (int)can_id, can_id & mask1, id1 & mask1);

    return 0;
}

[kevinevans]

Huh. Maybe I'm really misunderstanding this.

If I do:

Code: Select all

    id1=id2=0;
    mask1=mask2=0xFFFF;

    twai_mask_filter_config_t dual_config = twai_make_dual_filter(
        id1, mask1,
        id2, mask2,
        true
    );

Doesn't this mean both filters require the IDs to be 0? And this should let no messages through? I'm currently seeing frames come through with nonzero CAN identifiers. For example, I'm seeing CAN ID 9f50302 come through just fine.

Edit: opened a bug report here https://github.com/espressif/esp-idf/issues/17504

[kevinevans]

For anyone following along at home, there seems to be two independent bugs: (1) twai_make_dual_filter doesn't work correctly for extended IDs, (2) twai_node_config_mask_filter (or a hal function) doesn't seem to work for extended IDs (or maybe dual filters with extended IDs).

My temporary fix is just writing to the registers directly:

Code: Select all

static esp_err_t set_dual_filter_registers(
    uint16_t id1,
    uint16_t mask1,
    uint16_t id2,
    uint16_t mask2
) {
    // TWAI base addr: 0x6002B000

    volatile uint32_t * const mode = ((uint32_t *)(DR_REG_TWAI_BASE + 0x0000));
    volatile uint32_t * const acr = ((uint32_t *)(DR_REG_TWAI_BASE + 0x0040));
    volatile uint32_t * const amr = ((uint32_t *)(DR_REG_TWAI_BASE + 0x0050));

    if ((*mode & 0x1) == 0) {
        // must be in rst mode before setting any of these registers
        // mode[0:0] = {1: TWAI_RESET_MODE, 0: operating}
        return ESP_ERR_INVALID_STATE;
    }

    // clear the bit, i.e. set the filter mode to dual filter
    *mode = (*mode) & ~(0b1000);

    // invert the masks
    mask1 = ~mask1;
    mask2 = ~mask2;

    // filter 1
    acr[0] = (id1 >> 8) & 0xFF;
    acr[1] = (id1 & 0xFF);

    amr[0] = (mask1 >> 8) & 0xFF;
    amr[1] = (mask1 & 0xFF);

    // filter 2
    acr[2] = (id2 >> 8) & 0xFF;
    acr[3] = (id2 & 0xFF);

    amr[2] = (mask2 >> 8) & 0xFF;
    amr[3] = (mask2 & 0xFF);

    return ESP_OK;
}

Explanation here: https://github.com/espressif/esp-idf/is ... 3237494109

[MicroController]

Looks good.
You should make the registers volatile though (like "volatile uint32_t* const mode = ...").