mysmartgrid / hexabus

The HexaBus Project: An IPv6-based home automation bus. We develop both hard- and software for the future of home automation.
http://hexabus.net
49 stars 15 forks source link

CRC Kermit might be wrong implemented #227

Closed morriswinkler closed 9 years ago

morriswinkler commented 10 years ago

I am in the way of porting libhexabus to the go programming language, i just figured that the crc might not be a standard 16 bit Kermit.

From what i reed the two crc bytes have to be swapped after calculating the checksum.

so for example i have a package looking like

0x48 0x58 0x30 0x43 0x04 0x00 0x00 0x00 0x00 0x0a 0x01 0x00

libhexabus will add the checksum

0x6f 0x99

while the proper implementation should return

0x99 0x6f

please proof me wrong, if not this could be a boost problem

here my code

func crc16_KERMIT(packet []byte) uint16 {
    var crc uint16
        for _, v := range packet {
                crc = crc ^ uint16(v)
                for y := 0; y < 8; y++ {
                        if (crc & 0x001) == 0x0001 {
                                crc = (crc >> 1) ^ 0x8408
                        } else {
                                crc = crc >> 1
                        }
                }
        }
    // in the original Kermit implementation the two crc bytes are swaped, looks like boost::crc_optimal<16, 0x1021, 0x0000, 0, true, true> doesn't follow that ?
        //lb := (crc & 0xff00) >> 8
        //hb := (crc & 0x00ff) << 8
        //crc = hb | lb
    return crc
} 

as you can see i commented the last part to don't swap those two bytes ...

ghost commented 10 years ago

Why would you swap the bytes in your CRC routine? You only have to care about byte order when writing or reading a multibyte entity. In Hexabus packets, multibyte entities are big endian.

morriswinkler commented 10 years ago

Please look on crc16 Kermit implementations, they all swap the two remaining bits.

see for example http://stackoverflow.com/questions/4455257/crc16-checksum-hcs08-vs-kermit-vs-xmodem

Answer 2 explains it there is an option to swap the result and that is true for kermit.

ghost commented 10 years ago

You're right, it does say so there. I think the original author of the code got a little confused, some research into what checksum is actually used revealed that it's not Kermit. It's ITU-T, exactly as used in 802.15.4 frames for full-frame error detection.

morriswinkler commented 10 years ago

Can you please show me a reference to that specific crc type, i would like to change the wiki corresponding to that.

ghost commented 9 years ago

Sorry for forgetting this. We are close™ to merging a new version of the protocol that does away with the CRCs entirely now, so this problem will go away. (The CRCs never fulfilled their intended purpose anyway and, if anything, made it worse.)