ignore 3of6 decoding errors, let CRC fail decoding if necessary,

[klohner]

Addresses No bits decoded from wmbus T1 Message #2861

[zuckschwerdt]

does this handle nibble_h > 0xf?
Shouldn't the goal be to return the number of successful decoded bytes instead of just an error (or hiding the error)?

[zuckschwerdt]

Ok, I see it does. We compose a partial byte filling the broken nibble with zero.
Should we end the decoding on first error or keep going?

[klohner]

I thought it redundant to do nibble_h &= 0x0F; since the high part of that byte is getting immediately shifted away in output[n] = (nibble_h << 4) | nibble_l; (thus hiding any error in nibble_h as a side effect)

I agree it'd be more useful to have the return code be the number of decoded bytes found before the end or first error detected (instead of just 0 on success or -1 on failure), though that change may require further changes.

[klohner]

Are you thinking something like:

static int m_bus_decode_3of6_buffer(uint8_t const *bits, unsigned bit_offset, uint8_t* output, unsigned num_bytes)
{
    for (unsigned n=0; n<num_bytes; ++n) {
        uint8_t nibble_h = m_bus_decode_3of6(bitrow_get_byte(bits, n*12+bit_offset) >> 2);
        uint8_t nibble_l = m_bus_decode_3of6(bitrow_get_byte(bits, n*12+bit_offset+6) >> 2);
        if (nibble_h > 0xf || nibble_l > 0xf) {
            // return -1;  // fail at first 3of6 decoding error
            // nibble_l &= 0x0F;  // assume logical 0 nibble if 3of6 decoding error, let CRC fail decoding if necessary
            return n;  // return all successfully decoded bytes found until the first error
        }
        output[n] = (nibble_h << 4) | nibble_l;
    }
    return num_bytes;
}

It seems useful to me, but it looks like any non-negative result is then just categoriezed as success and then unused. Should we change that behavior as well?

[klohner]

I think a case could also be made to return the number of successful bytes until the first decode error (or even all bytes, regardless), still decoding all available bytes, zeroing any decoding errors, and let the caller decide if it's worth looking beyond the first decode error for other packets of data or try some kind of error correction. Though perhaps the returns on this effort would be too diminished to be useful.

[merbanan]

Fine by me.

[zuckschwerdt]

I don't think 3of6 lends itself well to EC, e.g. assume a single bit error in 001111, would this be 001101, 001110, or 001011?

[merbanan]

Well the CRC will probably only pass with one of these values. But the complexity of doing EC 2 steps back will get quite complicated quickly. In practice all 6-bit codes that are invalid has a valid value that is most likely to be the correct one. So if one had lots of time to invest into this all kinds of clever heuristics could be developed.

[merbanan]

I do think keeping track of nibble indices that 6to4 decodes improperly and then brute force trying all 4-bit combinations and calculating the CRC over that would be just as effective at doing EC compared to some clever heuristics in the 6to4 decoder.

[klohner]

Do we need to implement EC as part of this change?