tfmini icon indicating copy to clipboard operation
tfmini copied to clipboard

Include the last frame byte in the checksum calculation

Open bloveless opened this issue 5 years ago • 8 comments

Fixed checksum to include the last byte which is the temp H byte.

I was getting calculations such as the following.

Starting checksum: 178
0: adding 207 - new checksum: 129
1: adding 0 - new checksum: 129
2: adding 11 - new checksum: 140
3: adding 35 - new checksum: 175
4: adding 208 - new checksum: 127
----------------------------------------
i: 0: 207
i: 1: 0
i: 2: 11
i: 3: 35
i: 4: 208
i: 5: 9
i: 6: 136
127 = 136

You'll notice that the final checksum is 127 rather than 136 and that when calculating the checksum that frame[5] is missing. I've changed the checksum calculation to include this final byte.

After a little research, it seems that the tfmini and the tfmini-s use the same number of bytes in the frame but the tfmini-s use the last two bytes for the temperature. Either way, this change makes this library compatible with the tfmini-s as well as the tfmini.

bloveless avatar Sep 13 '20 17:09 bloveless

Hi bloveless, thanks for this contribution. I don't have a tfmini-s handy, is this tested and verified working on both the tfmini and tfmini-s? My workbench is a little crazy right now (I've been taking care of our toddler during quarantine for 6 months), so it's not easy for me to test right now, and I don't want to risk breaking the driver if it's not yet tested on both. thanks, Peter

PeterAJansen avatar Sep 14 '20 05:09 PeterAJansen

I don’t have a TFMini but after I got this driver to work I realized that only the distance measurement frame is the same. None of the commands work on the TFMini-S. So I’ll leave it up to you if you want to close this PR. I’m going to make a new driver for the TFMini-S based on your driver.

bloveless avatar Sep 14 '20 11:09 bloveless

But thank you for making this driver! It has really made my development easier for the TFMini-S driver!

bloveless avatar Sep 14 '20 11:09 bloveless

Thanks -- I've had a few requests for the TFMini-S, but given how crazy the world is right now haven't had the time to pick one up and do the verification.

I think the changes are fairly minor to have the same codebase work on both (with a #DEFINE or similar that picks out which version to use) -- but maybe the best thing is to break it out into two separate drivers so that new users have something that just works out of the box without it being potentially confusing to have to set the define.

PeterAJansen avatar Sep 14 '20 18:09 PeterAJansen

I'll try and remember to come back here when I'm done with my driver and maybe we can team up to provide a unified driver for both! Either that or we can just crosslink the projects.

bloveless avatar Sep 14 '20 20:09 bloveless

I got most of the tfmini-s driver working last night if you want to take a look. https://github.com/bloveless/tfmini-s

bloveless avatar Sep 14 '20 20:09 bloveless

can this be pulled? @PeterAJansen

Bjohnson131 avatar Oct 26 '20 17:10 Bjohnson131

@tuskiomi If you are trying to support the TFMini-S as I was trying to do with this PR then you might try out my driver specifically for the TFMini-S. There are a few more modifications that are necessary to support TFMini-S correctly and this change only allows you to read the height on a TFMini-S. None of the management commands work even if this PR is applied.

bloveless avatar Oct 26 '20 19:10 bloveless