hardkernel / smartpower3

11 stars 6 forks source link

Some data comes without channel #1 fields #16

Closed tomek-szczesny closed 1 year ago

tomek-szczesny commented 2 years ago

I noticed that today with up-to-date firmware, at 1000ms intervals via ttyUSB:

0094016085,19143,0055,01052,0,05011,0015,00075,1,00,00005,0000,00000,0,00,f8,14
0094017084,19142,0056,01071,0,05016,0014,00070,1,00,00004,0000,00000,0,00,f9,15
0094018084,19146,0055,01053,0,05011,0014,00070,1,00,00006,0000,00000,0,00,f8,18
0094019085,19143,0056,01072,0,05013,0014,00070,1,00,00005,0000,00000,0,00,f6,1c
0094020084,19143,0056,01072,0,05014,0015,00075,1,00,00006,0000,00000,0,00,f7,17
0094021085,19145,0055,01053,0,00004,0000,00000,0,00,100,12
0094022084,19144,0056,01072,0,05013,0014,00070,1,00,00006,0000,00000,0,00,fb,11
0094023084,19139,0055,01052,0,05014,0014,00070,1,00,00006,0000,00000,0,00,f8,1c
0094024084,19142,0055,01052,0,00005,0000,00000,0,00,100,12
0094025085,19145,0055,01052,0,05013,0014,00070,1,00,00005,0000,00000,0,00,fa,14
0094026085,19143,0055,01052,0,05015,0015,00075,1,00,00004,0000,00000,0,00,f4,12
0094027084,19141,0055,01052,0,05015,0014,00070,1,00,00006,0000,00000,0,00,fa,16
0094028085,19143,0055,01052,0,05014,0015,00075,1,00,00006,0000,00000,0,00,f1,1f
0094029084,19143,0055,01052,0,05011,0014,00070,1,00,00005,0000,00000,0,00,fb,1d
0094030085,19145,0055,01052,0,05013,0015,00075,1,00,00005,0000,00000,0,00,f8,14
0094031084,19144,0056,01072,0,05010,0015,00075,1,00,00005,0000,00000,0,00,f9,17
0094032084,19143,0055,01052,0,05013,0014,00070,1,00,00005,0000,00000,0,00,ff,15
0094033085,19143,0055,01052,0,05013,0014,00070,1,00,00005,0000,00000,0,00,fd,15
0094034085,19145,0055,01053,0,05013,0014,00070,1,00,00005,0000,00000,0,00,f9,15
0094035084,19143,0055,01052,0,05015,0014,00070,1,00,00005,0000,00000,0,00,fa,14
0094036085,19143,0055,01052,0,05010,0015,00075,1,00,00005,0000,00000,0,00,f7,17
0094037084,19139,0055,01052,0,05013,0014,00070,1,00,00004,0000,00000,0,00,f6,1c
0094038084,19144,0055,01052,0,05013,0014,00070,1,00,00006,0000,00000,0,00,f7,1b
0094039084,19138,0055,01052,0,05013,0015,00075,1,00,00005,0000,00000,0,00,ee,16
0094040085,19147,0055,01053,0,05011,0015,00075,1,00,00005,0000,00000,0,00,f6,12
0094041085,19142,0056,01071,0,05012,0014,00070,1,00,00006,0000,00000,0,00,fd,11
0094042084,19140,0055,01052,0,00006,0000,00000,0,00,100,12
0094043084,19144,0055,01052,0,05011,0014,00070,1,00,00006,0000,00000,0,00,fd,15
0094044084,19141,0055,01052,0,05010,0014,00070,1,00,00005,0000,00000,0,00,01,15
0094045085,19142,0055,01052,0,05013,0014,00070,1,00,00006,0000,00000,0,00,fa,16
0094046084,19145,0056,01072,0,05013,0014,00070,1,00,00004,0000,00000,0,00,f6,10
0094047085,19143,0056,01072,0,05013,0014,00070,1,00,00005,0000,00000,0,00,f5,17
0094048084,19146,0055,01053,0,05013,0015,00075,1,00,00004,0000,00000,0,00,ef,19
0094049085,19142,0056,01071,0,05014,0014,00070,1,00,00005,0000,00000,0,00,f4,1c
0094050085,19148,0055,01053,0,05014,0015,00075,1,00,00004,0000,00000,0,00,f2,18

There is no visible pattern how often it happens, and the last number is not always 12. I've seen 10, 12, 14, 16, 18 and 1a. The number "100" is suspicious and always is there if error happens.

https://github.com/hardkernel/smartpower3/blob/933acc1c3775d7c108ffab2ebf82df65e639f60e/smartpower3/smartpower3.ino#L113 I'm worried that "(byte)(~checksum8)+1" may actually return 0x100 if checksum8 == 0x00.

Still don't know why would it influence data sent in the past.

declanmalone commented 2 years ago

Looks like a buffer overflow. The buffer_checksum array is 8 bytes. Just enough for "xx,yy\r\n" and a terminating \0 that sprintf() adds. These buffers are stack-allocated, with the channel1 array directly after it in memory. The sprintf is thus writing \0 to the start of that string array, so nothing gets printed.

Fixing the cast should solve the problem, I think:

sprintf(buffer_checksum, "%02x,%02x\r\n", (byte)((~checksum8)+1), checksum8_xor);
tomek-szczesny commented 2 years ago

I'm both grateful for your explanation and proud of myself I found that bug myself. Thanks a lot!

cedel1 commented 1 year ago

PR: https://github.com/hardkernel/smartpower3/pull/27

cedel1 commented 1 year ago

Fixed in release 2.0 (https://wiki.odroid.com/accessory/power_supply_battery/smartpower3#apr03_v20).