search

Openvario / sensord

Daemon to poll air data from Openvario sensorboard
6 stars 11 forks source link

Reverted changes in update_checksum and verify_checksum #44

Closed iglesiasmarianod closed 2 years ago

iglesiasmarianod commented 2 years ago

fixes openvario/meta-openvario#285

MaxKellermann commented 2 years ago

This revert is wrong, do not merge! It does not "fix" anything, it only makes a symptom go away, but ignores the reason for for this symptom (and doesn't explain anything in the commit message).

sizeof(data) == 4, so this checksums only the first 4 bytes of this struct (i.e. the 3-byte header plus the version field). If it were correct to only checksum the first 4 bytes, it wouldn't use the size of a pointer for this (which is 8 on 64 bit CPUs).

Note that there are two padding bytes in this struct; the way it was initially authored is pretty clumsy. This padding could maybe cause the problem (just a random guess).

MaxKellermann commented 2 years ago

I have a better guess what's the problem here: the "crc" field is included in the CRC calculation. This was not a problem before because only the first 4 bytes were used before. Both is buggy, my recent change only made this problem visible.

iglesiasmarianod commented 2 years ago

Thanks @MaxKellermann for checking this. Do you want me to close the PR? I already went back to your changes and will test uncommenting the outputs.

linuxianer99 commented 2 years ago

@iglesiasmarianod Think the problem is in update_checksum function in 24c16.c. As Max mentioned, the checksum is included in the checksum ;-) I will have a look on this ... You can close this PR ...

iglesiasmarianod commented 2 years ago

Thanks @linuxianer99 I'll close it. If you need to test I have a complete 5.7" OV at home.