search

baryluk / fnirsi-usb-power-data-logger

Driver / Data logger for FNIRSI FNB48, FNIRSI C1 and FNIRSI FNB58 USB Power meter
MIT License
141 stars 14 forks source link

CRC module update possibly breaks the logger #12

Closed parkerlreed closed 10 months ago

parkerlreed commented 1 year ago

python 3.10.7 python3 crc 4.2.0

# ./fnirsi_logger.py
Traceback (most recent call last):
  File "/root/fnirsi-usb-power-data-logger/./fnirsi_logger.py", line 242, in <module>
    main()
  File "/root/fnirsi-usb-power-data-logger/./fnirsi_logger.py", line 154, in main
    crc_calculator = setup_crc()  # can be None
  File "/root/fnirsi-usb-power-data-logger/./fnirsi_logger.py", line 51, in setup_crc
    crc_calculator = crc.CrcCalculator(configuration, use_table)
AttributeError: module 'crc' has no attribute 'CrcCalculator'. Did you mean: 'Calculator'?
parkerlreed commented 1 year ago

Looking through pypi crc releases and I don't see any of them with the CrcCalculator function. Could this have possibly been a different module?

parkerlreed commented 1 year ago

Aha crc 1.2.0 is the last to have CrcCalculator. This is 7 releases behind current. Downgraded crc to that version and the logger works again.

baryluk commented 1 year ago

It should also work with crc 1.3.x. If you look at crc library changelog it was changed in 2.x https://nicoretti.github.io/crc/changelog/changes_2.0.0/ from CrcCalculator to Calculator.

The best is to use requirements.txt file included in this repo to ensure proper version.

$ pip3 install -r requirements.txt

This looks like something very easy to fix to support new version, or even both old and new. Patches welcome.

crc library is optional, and not strictly required. If you uninstall it, script should work and just ignore CRC checks

I might look into this at some point to also use other library, like crcmod ( https://crcmod.sourceforge.net/crcmod.html ), as this one is available on many distros without needing to use pip).

parkerlreed commented 1 year ago

Oh derp thanks. I completely forgot requirements.txt was a thing :D

Thank you!

parkerlreed commented 1 year ago

Using 4.2.0 and a FNB48S produces a ton of invalid checksums. Works as expected on 1.3.0

(deck@steamdeck fnirsi-usb-power-data-logger)$ ./fnirsi_logger.py 

timestamp sample_in_packet voltage_V current_A dp_V dn_V temp_C_ema energy_Ws capacity_As
Ignoring packet of length 64 with unexpected checksum. Expected: 42 Actual: 28
Ignoring packet of length 64 with unexpected checksum. Expected: aa Actual: 71
Ignoring packet of length 64 with unexpected checksum. Expected: 31 Actual: e8
Ignoring packet of length 64 with unexpected checksum. Expected: 55 Actual: d9
Ignoring packet of length 64 with unexpected checksum. Expected: 31 Actual: 6e
Ignoring packet of length 64 with unexpected checksum. Expected: 46 Actual: 43
Ignoring packet of length 64 with unexpected checksum. Expected: 46 Actual: b4
Ignoring packet of length 64 with unexpected checksum. Expected: 84 Actual: b0
Ignoring packet of length 64 with unexpected checksum. Expected: 84 Actual: 25
Ignoring packet of length 64 with unexpected checksum. Expected: 23 Actual: 1d
Ignoring packet of length 64 with unexpected checksum. Expected: 46 Actual: 41
Ignoring packet of length 64 with unexpected checksum. Expected: 6d Actual: b5
Ignoring packet of length 64 with unexpected checksum. Expected: 46 Actual: 57
Ignoring packet of length 64 with unexpected checksum. Expected: 62 Actual: 49
Ignoring packet of length 64 with unexpected checksum. Expected: 62 Actual: fa
Ignoring packet of length 64 with unexpected checksum. Expected: da Actual: 64
Ignoring packet of length 64 with unexpected checksum. Expected: 42 Actual: d4
Ignoring packet of length 64 with unexpected checksum. Expected: 42 Actual: 1f
Ignoring packet of length 64 with unexpected checksum. Expected: 31 Actual: 3a
Ignoring packet of length 64 with unexpected checksum. Expected: aa Actual: da
Ignoring packet of length 64 with unexpected checksum. Expected: b1 Actual: e1
Ignoring packet of length 64 with unexpected checksum. Expected: 31 Actual: 26
Ignoring packet of length 64 with unexpected checksum. Expected: 31 Actual: 09
Ignoring packet of length 64 with unexpected checksum. Expected: b6 Actual: a7
Ignoring packet of length 64 with unexpected checksum. Expected: da Actual: ab
Ignoring packet of length 64 with unexpected checksum. Expected: 23 Actual: f4
Ignoring packet of length 64 with unexpected checksum. Expected: b1 Actual: 34
Ignoring packet of length 64 with unexpected checksum. Expected: aa Actual: e8
Ignoring packet of length 64 with unexpected checksum. Expected: 5b Actual: d8
Ignoring packet of length 64 with unexpected checksum. Expected: 46 Actual: 16
Ignoring packet of length 64 with unexpected checksum. Expected: b1 Actual: 73
Ignoring packet of length 64 with unexpected checksum. Expected: 8c Actual: 31
Ignoring packet of length 64 with unexpected checksum. Expected: 23 Actual: 7c
Ignoring packet of length 64 with unexpected checksum. Expected: da Actual: ae
Ignoring packet of length 64 with unexpected checksum. Expected: 46 Actual: 26
Ignoring packet of length 64 with unexpected checksum. Expected: da Actual: 66
Ignoring packet of length 64 with unexpected checksum. Expected: da Actual: e7
Ignoring packet of length 64 with unexpected checksum. Expected: 6d Actual: ef
Ignoring packet of length 64 with unexpected checksum. Expected: da Actual: 93
Ignoring packet of length 64 with unexpected checksum. Expected: da Actual: 11
Ignoring packet of length 64 with unexpected checksum. Expected: 46 Actual: 8f
Ignoring packet of length 64 with unexpected checksum. Expected: b1 Actual: 8d
Ignoring packet of length 64 with unexpected checksum. Expected: 5b Actual: 0c
Ignoring packet of length 64 with unexpected checksum. Expected: 55 Actual: 64
Ignoring packet of length 64 with unexpected checksum. Expected: 21 Actual: 06
Ignoring packet of length 64 with unexpected checksum. Expected: 23 Actual: 06
Ignoring packet of length 64 with unexpected checksum. Expected: 6d Actual: ab
Ignoring packet of length 64 with unexpected checksum. Expected: da Actual: f3
Ignoring packet of length 64 with unexpected checksum. Expected: b1 Actual: d0
Ignoring packet of length 64 with unexpected checksum. Expected: 55 Actual: b9
1684179096.684 0 0.09877 0.00049 1.798 1.863 23.200 0.000000 0.000005
1684179096.694 1 0.09877 0.00046 1.798 1.863 23.200 0.000001 0.000010
1684179096.704 2 0.09861 0.00052 1.798 1.863 23.200 0.000001 0.000015
1684179096.714 3 0.09893 0.00049 1.798 1.863 23.210 0.000002 0.000020
Ignoring packet of length 64 with unexpected checksum. Expected: b1 Actual: aa
Ignoring packet of length 64 with unexpected checksum. Expected: b1 Actual: 15
Ignoring packet of length 64 with unexpected checksum. Expected: 31 Actual: 9a
Ignoring packet of length 64 with unexpected checksum. Expected: da Actual: e6
luismeruje commented 10 months ago

I've set CRC checking as an adjustable configuration in this pull request since it does not work correctly for all devices .

baryluk commented 10 months ago

So, the code now does not check crc by default. (It can be enabled by passing --crc=true on command line).

I was able to reproduce the issues on my FNB48, FNB58 and C1, when using crc 5.0.0

It works fine with crc 1.2.0, crc 1.3.0, crc 2.0.0

With crc 3.0.x, 4.0.0, 4.1.0, it breaks (TypeError: 'int' object is not iterable)

With crc 4.2.0, it sporadically (little higher than chance) is correct.

crc 4.3.0 an 5.0.0 is essentially never is correct.

Looks like a bug in crc library to me.