tpircher-zz / pycrc

Free, easy to use Cyclic Redundancy Check (CRC) calculator and source code generator
https://pycrc.org
MIT License
169 stars 36 forks source link

different CRC calculated using bbf and table driven with idx-width = 4 on stm8 platform #18

Closed krzysztofkuczek closed 7 years ago

krzysztofkuczek commented 7 years ago

Hi, I have experienced problem with using table driven algorithm on 8bit machine. It returns different crc values then bbf calculates. Same piece of code running on win32 platform returns correct (the same) crc. What should i do to make it works? I'm using stm8 platform with Cosmic free compiler. Thank you in advance Krzysiek

tpircher-zz commented 7 years ago

Hi Krzysiek, can you let me know the CRC parameters you are using to generate the source please? What version of pycrc are you using?

krzysztofkuczek commented 7 years ago

Hi, I did it this way pycrc.py --model crc-16 --algorithm bbf --generate h -o crc.h pycrc.py --model crc-16 --table-idx-width 4 --algorithm table-driven --generate h -o crc.h

The code looks like following:

for table driven approach:

static const crc_t crc_table2[16] = {
    0x0000, 0xcc01, 0xd801, 0x1400, 0xf001, 0x3c00, 0x2800, 0xe401,
    0xa001, 0x6c00, 0x7800, 0xb401, 0x5000, 0x9c01, 0x8801, 0x4400
};

crc_t crc_update2(crc_t crc, const void *data, size_t data_len)
{
    const unsigned char *d = (const unsigned char *)data;
    unsigned int tbl_idx;

    while (data_len--) {
        tbl_idx = crc ^ (*d >> (0 * 4));
        crc = crc_table2[tbl_idx & 0x0f] ^ (crc >> 4);
        tbl_idx = crc ^ (*d >> (1 * 4));
        crc = crc_table2[tbl_idx & 0x0f] ^ (crc >> 4);

        d++;
    }
    return crc & 0xffff;
}

for bbf

crc_t crc_update(crc_t crc, const void *data, size_t data_len)
{
    const unsigned char *d = (const unsigned char *)data;
    unsigned int i;
    bool bit;
    unsigned char c;

    while (data_len--) {
        c = *d++;
        for (i = 0x01; i & 0xff; i <<= 1) {
            bit = crc & 0x8000;
            if (c & i) {
                bit = !bit;
            }
            crc <<= 1;
            if (bit) {
                crc ^= 0x8005;
            }
        }
        crc &= 0xffff;
    }
    return crc & 0xffff;
}

and what should not be important i manually changed crc_init's return value.

crc_t @inline crc_init(void)
{
    return 0xA1F2;
}

Regards Krzysiek

tpircher-zz commented 7 years ago

Ok, confirmed. Tested with the online msp430 simulator (http://www.msp430emulator.com) and the following project:

target=firmware.elf
src=main.c
gen=crc_bbf.c crc_tbl.c
obj=$(src:.c=.o) $(gen:.c=.o)

CC=msp430-gcc
LDFLAGS=-mmcu=msp430g2553 -g

all: $(target)

$(target): $(obj)
        $(CC) $(LDFLAGS) -o $@ $^

main.o: main.c crc_bbf.h crc_tbl.h
crc_bbf.h:
        pycrc.py --symbol-prefix crc_bbf_ --model crc-16 --xor-in 0xa1f2 --algorithm bbf --generate h -o crc_bbf.h
crc_bbf.c:
        pycrc.py --symbol-prefix crc_bbf_ --model crc-16 --xor-in 0xa1f2 --algorithm bbf --generate c -o crc_bbf.c

crc_tbl.h:
        pycrc.py --symbol-prefix crc_tbl_ --model crc-16 --xor-in 0xa1f2 --algorithm tbl --table-idx-width 4 --generate h -o crc_tbl.h
crc_tbl.c:
        pycrc.py --symbol-prefix crc_tbl_ --model crc-16 --xor-in 0xa1f2 --algorithm tbl --table-idx-width 4 --generate c -o crc_tbl.c

.PHONY: clean
clean:
        $(RM) $(gen) $(obj) $(target) crc_bbf.h crc_tbl.h

and the main.c file

#include <stdio.h>
#include "crc_tbl.h"
#include "crc_bbf.h"

void xprintf(const char *fmt, unsigned int val)
{
    (void)fmt;
    volatile unsigned int xxx = val;
}

int main(void)
{
    crc_tbl_t res_tbl = crc_tbl_init();
    res_tbl = crc_tbl_update(res_tbl, "123456789", 9);
    res_tbl = crc_tbl_finalize(res_tbl);
    xprintf("tbl: %#x\n", res_tbl);

    crc_bbf_t res_bbf = crc_bbf_init();
    res_bbf = crc_bbf_update(res_bbf, "123456789", 9);
    res_bbf = crc_bbf_finalize(res_bbf);
    xprintf("bbf: %#x\n", res_bbf);
}

The bbf algorithm (correctly) calculates 0x5d27, the tbl algorithm yields the wrong result 0x42e8.

krzysztofkuczek commented 7 years ago

Hello Thomas, Thanks for confirmation. I also made observation, that changing single byte does not change CRC calculated by table driven algorithm.

Unfortunately it happens only on stm8 environment. I try to confirm that using msp430emulator, but without success, so maybe it's not like I think... So, this data produce same CRC values, when calculation is done on stm8 machine.

 uint8_t testData[] = { 0x51, 0x63, 0x7A, 0x65, 0x6B, 0x5F, 0x4C, 0x52, 0x53, 0x5F, 0x73, 0x6C, 0x61, 0x76, 0x65, 0x5F,
                        0x76, 0x2E, 0x5F, 0x30, 0x2E, 0x39, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
                        0x00, 0x00, 0xFF, 0xFF, 0x08, 0x00, 0x00, 0x00, 0x04, 0x0F, 0x00, 0x00, 0xE1, 0x00, 0x00, 0x19,
                        0xCF, 0x0E, 0x40, 0x19, 0xD0, 0xF6, 0x88, 0x19,      0xD2,      0xDE, 0xD0, 0x19, 0xD4, 0xC7, 0x18, 0x19,
                        0xD6, 0xAF, 0x60, 0x19, 0xD8, 0x97, 0xA8, 0x19, 0xDA, 0x7F, 0xF0, 0x19, 0xDC, 0x68, 0x38 };

 uint8_t testData2[] = { 0x51, 0x63, 0x7A, 0x65, 0x6B, 0x5F, 0x4C, 0x52, 0x53, 0x5F, 0x73, 0x6C, 0x61, 0x76, 0x65, 0x5F,
                        0x76, 0x2E, 0x5F, 0x30, 0x2E, 0x39, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
                        0x00, 0x00, 0xFF, 0xFF, 0x08, 0x00, 0x00, 0x00, 0x04, 0x0F, 0x00, 0x00, 0xE1, 0x00, 0x00, 0x19,
                        0xCF, 0x0E, 0x40, 0x19, 0xD0, 0xF6, 0x88, 0x19,      0xD1,      0xDE, 0xD0, 0x19, 0xD4, 0xC7, 0x18, 0x19,  // D1 instead of D2
                        0xD6, 0xAF, 0x60, 0x19, 0xD8, 0x97, 0xA8, 0x19, 0xDA, 0x7F, 0xF0, 0x19, 0xDC, 0x68, 0x38 };

Btw, When are you going to release 0.9.1 :)?

tpircher-zz commented 7 years ago

Hi Krzysiek, I must have misread your original post and have mixed up msp430 with stm8 sorry (I shouldn't attempt to fix bugs late at night...). Do you know any free stm8 emulator where I can run some code? Regarding v0.9.1, I admit I have been lazy with pycrc lately. I wanted to streamline some bbf algorithms before that release but haven't had the time to do so..

tpircher-zz commented 7 years ago

One more thing, the issue I ran into with the msp430 appears to be a problem with the simulator (the right shift operation, to be precise (https://github.com/RudolfGeosits/MSP430-Emulator/issues/7)), so I'm back to square one.

krzysztofkuczek commented 7 years ago

Hi Thomas, I think you can use ST Visual Develop and Cosmic compiler for simulation. Another strange thing I found, is that bbf algoritm, calculates crc=0 for "zeros" data, even if previously calculated crc passed as first parameters is not zero. Is it ok? See screenshots below.

crc_step1 crc_step2

Best regards Krzysiek

tpircher-zz commented 7 years ago

Hi Krzysiek,

I cannot confirm the bug. I have installed the ST Visual Develop and Cosmic compiler. I compile for the STM8AF5168 chip (the first one in the list). The pycrc code is generated like this:

pycrc.py --symbol-prefix crc_tbl_ --std c89 --model crc-16 --xor-in 0xa1f2 --algorithm tbl --generate h -o crc_tbl.h
pycrc.py --symbol-prefix crc_tbl_ --std c89 --model crc-16 --xor-in 0xa1f2 --algorithm tbl --generate c -o crc_tbl.c

Note the --std c89 option, as the compiler seems not to understand C99.

My main function is this:

#include <crc_tbl.h>

typedef unsigned char uint8_t;

void xprintf(const char *fmt, unsigned int val)
{
    volatile unsigned int xxx = val;
    (void)fmt;
    (void)xxx;
}

uint8_t testData[] = { 0x51, 0x63, 0x7A, 0x65, 0x6B, 0x5F, 0x4C, 0x52, 0x53, 0x5F, 0x73, 0x6C, 0x61, 0x76, 0x65, 0x5F,
    0x76, 0x2E, 0x5F, 0x30, 0x2E, 0x39, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
    0x00, 0x00, 0xFF, 0xFF, 0x08, 0x00, 0x00, 0x00, 0x04, 0x0F, 0x00, 0x00, 0xE1, 0x00, 0x00, 0x19,
    0xCF, 0x0E, 0x40, 0x19, 0xD0, 0xF6, 0x88, 0x19,      0xD2,      0xDE, 0xD0, 0x19, 0xD4, 0xC7, 0x18, 0x19,
    0xD6, 0xAF, 0x60, 0x19, 0xD8, 0x97, 0xA8, 0x19, 0xDA, 0x7F, 0xF0, 0x19, 0xDC, 0x68, 0x38 };

uint8_t testData2[] = { 0x51, 0x63, 0x7A, 0x65, 0x6B, 0x5F, 0x4C, 0x52, 0x53, 0x5F, 0x73, 0x6C, 0x61, 0x76, 0x65, 0x5F,
    0x76, 0x2E, 0x5F, 0x30, 0x2E, 0x39, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
    0x00, 0x00, 0xFF, 0xFF, 0x08, 0x00, 0x00, 0x00, 0x04, 0x0F, 0x00, 0x00, 0xE1, 0x00, 0x00, 0x19,
    0xCF, 0x0E, 0x40, 0x19, 0xD0, 0xF6, 0x88, 0x19,      0xD1,      0xDE, 0xD0, 0x19, 0xD4, 0xC7, 0x18, 0x19,  // D1 instead of D2
    0xD6, 0xAF, 0x60, 0x19, 0xD8, 0x97, 0xA8, 0x19, 0xDA, 0x7F, 0xF0, 0x19, 0xDC, 0x68, 0x38 };

main()
{
    crc_tbl_t_ res_tbl = crc_tbl_init();
     res_tbl = crc_tbl_update(res_tbl, testData, sizeof(testData));
     res_tbl = crc_tbl_finalize(res_tbl);
     xprintf("tbl: %#x\n", res_tbl);

     res_tbl = crc_tbl_init();
     res_tbl = crc_tbl_update(res_tbl, testData2, sizeof(testData2));
     res_tbl = crc_tbl_finalize(res_tbl);
     xprintf("tbl: %#x\n", res_tbl);

     while (1);
}

I get for the first dataset 0x66f6 as result and for the second data set 0x83a2. I get the same results with the bbf algoritm.

The generated code works fine for me on the 8 bit platform. Can you post a minimal example that shows the bug?

krzysztofkuczek commented 7 years ago

Hello, Thank you for your support. First of all, now i know that missing --std c89 caused problems with bbf implementation. It was related to bool bit variable definition. When i generated crc routines using --sdt c89 bbf works fine. Thank you. But when i generate table based routines with --table-idx-widtyh = 4

pycrc.py --symbol-prefix crctab --std c89 --model crc-16 --xor-in 0xa1f2 --table-idx-width 4 --algorithm table-driven --generate c -o crc_tab.c pycrc.py --symbol-prefix crctab --std c89 --model crc-16 --xor-in 0xa1f2 --table-idx-width 4 --algorithm table-driven --generate h -o crc_tab.h

It produces different CRC then bbf implementation. You can download project i made to see how it works. https://drive.google.com/file/d/0B4nlJmp6QsrON2xnbmdQbG5iODg/view?usp=sharing

Best regards Krzysiek

tpircher-zz commented 7 years ago

Krzysiek, you have to use the _init() and _finalize() functions. If you look at the generated header files then you will see that the the bbf and tbl variants return different initial values. If you need to specify a custom init value, that's what the --xor-in 0xa1f2 option is for.

If you call init and finalize, then the bbf and tbl algorithms will return the same value.

krzysztofkuczek commented 7 years ago

ok, thank you ! So i need to call _finalize after every _update function call... Thank you. Krzysiek

tpircher-zz commented 7 years ago

Hi Krzysiek, no, you must call the _init() and _finalize() functions only once. The _init() funtion must be called before the first call to _update(), and the _finalize() function must be called after the last call to _update(), and before you use the result. In between the _init() and _finalize() you can call the _update() function any number of times, even 0 times. https://pycrc.org/tutorial.html has some examples.

krzysztofkuczek commented 7 years ago

ok, thank you.

krzysztofkuczek commented 7 years ago

It's me again. I didn't stop my investigation, and problem is related to bug in Cosmic compiler. It's confirmed by Cosmic support. You can find more detauls here https://community.st.com/message/154305-re-stm8s-cosmic-compiler-crc-calculation-problems?commentID=154305

Regards Krzysiek

tpircher-zz commented 7 years ago

Good spot! I'm glad this is sorted now!