> On Fri, Apr 7, 2017 at 12:27 PM, Andreas Klinger <ak@xxxxxxxxxxxxx> wrote: > > Hi Matt, > > > > i've extracted the error condition in a small userspace application for > > demonstration. Just compile and see that in this case the variable H3 is zero > > but the whole term evaluates differently if treated as unsigned or signed. > > > > The whole example including the values of the variables are taken from the real > > driver. > > > > Ok now I see what you are referencing from the datasheet, and can > confirm the values are incorrect. > > Just a suggestion rather than have all those casts it would be easier > to just change H1 and H3 types > to signed integers. > That's right. But i wanted to keep close to the documentation. > Thanks, > > Matt > > > > Andreas > > > > > > Now the example: > > > > --- > > > > /* > > * humidity.c > > * this test program is just for demonstrating the difference in the > > * calculation of humidity compensation with BME280 sensor > > * > > * This program is free software; you can redistribute it and/or modify > > * it under the terms of the GNU General Public License version 2 as > > * published by the Free Software Foundation. > > * > > */ > > > > #include <stdio.h> > > #include <stdlib.h> > > > > int main(int argn, char* argv[]) > > { > > int adc_humidity = 28275; > > int t_fine = 50623; > > unsigned int H1 = 75, H3 = 0; > > int H2 = 360, H4 = 324, H5 = 0, H6 = 30; > > int var; > > > > var = -26177; > > /* extracted errornous term with cast */ > > printf("with cast: %d\n", (((var * H6) >> 10) * (((var * (int)H3) >> 11) > > + (int)32768)) >> 10); > > /* extracted errornous term now without a cast */ > > printf("without cast: %d\n", (((var * H6) >> 10) * (((var * H3) >> 11) > > + 32768)) >> 10); > > > > printf("\n"); > > printf("t_fine: %d; humidity: %d\nH: %d; %d; %d; %d; %d; %d\n", > > t_fine, adc_humidity, H1, H2, H3, H4, H5, H6); > > printf("\n"); > > > > /* the whole example taken from the driver */ > > /* with the cast as proposed by the documentation */ > > var = ((int)t_fine) - (int)76800; > > var = ((((adc_humidity << 14) - (H4 << 20) - (H5 * var)) > > + (int)16384) >> 15) * (((((((var * H6) >> 10) > > * (((var * (int)H3) >> 11) + (int)32768)) >> 10) > > + (int)2097152) * H2 + 8192) >> 14); > > var -= ((((var >> 15) * (var >> 15)) >> 7) * (int)H1) >> 4; > > > > printf("with cast: var: %d\n", var>>12); > > printf("\n"); > > > > /* now the same calculation without the cast */ > > var = (t_fine) - 76800; > > var = ((((adc_humidity << 14) - (H4 << 20) - (H5 * var)) > > + 16384) >> 15) * (((((((var * H6) >> 10) > > * (((var * H3) >> 11) + 32768)) >> 10) > > + 2097152) * H2 + 8192) >> 14); > > var -= ((((var >> 15) * (var >> 15)) >> 7) * H1) >> 4; > > > > printf("without cast: var: %d\n", var>>12); > > > > return 0; > > } > > > > --- > > > > Output: > > > > with cast: -24544 > > without cast: 4169760 > > > > t_fine: 50623; humidity: 28275 > > H: 75; 360; 0; 324; 0; 30 > > > > with cast: var: 41671 > > > > without cast: var: 124497 > > > > --- > > > > Matt Ranostay <matt.ranostay@xxxxxxxxxxxx> schrieb am Thu, 06. Apr 23:13: > >> On Mon, Apr 3, 2017 at 11:16 PM, Matt Ranostay > >> <matt.ranostay@xxxxxxxxxxxx> wrote: > >> > On Sun, Apr 2, 2017 at 10:29 AM, Andreas Klinger <ak@xxxxxxxxxxxxx> wrote: > >> >> Linus Walleij <linus.walleij@xxxxxxxxxx> schrieb am Sun, 02. Apr 16:56: > >> >>> On Sun, Apr 2, 2017 at 11:32 AM, Jonathan Cameron <jic23@xxxxxxxxxx> wrote: > >> >>> > On 27/03/17 11:06, Andreas Klinger wrote: > >> >>> >> While calculating the compensation of the humidity there are negative > >> >>> >> values interpreted as unsigned because of unsigned variables used. > >> >>> >> These values need to be casted to signed as indicated by the documentation > >> >>> >> of the sensor. > >> >>> >> > >> >>> >> Signed-off-by: Andreas Klinger <ak@xxxxxxxxxxxxx> > >> >>> > > >> >>> > Looks superficially right to me, but would like an Ack from Linus Walleij. > >> >>> > >> >>> I didn't work on these calculations, only infrastructure for the driver > >> >>> but FWIW: > >> >>> Acked-by: Linus Walleij <linus.walleij@xxxxxxxxxx> > >> >>> > >> >>> Matt Ranostay @Intel is the person you probably want to ACK it though, > >> >>> he added the humidity calculations :) > >> >> > >> >> It just turned out that there must be another error in the calculation. I'll fix > >> >> and test it first. > >> >> > >> >> When i'm done i'll send a new version of the patch. > >> >> > >> > > >> > Ok will hold off till review till the next patchset... BTW this > >> > datasheet's calculations made my eyes bleed so it is quiet likely it > >> > has bugs... > >> > >> Hmm the datatypes you are casting to s32 are already signed > >> integers... Interested in what you found on what the actual bug is. > >> > >> > > >> > Thanks, > >> > > >> > Matt > >> > > >> >> Andreas > >> >> > >> >>> > >> >>> Yours, > >> >>> Linus Walleij > >> >> > >> >> -- > >> >> -- > >> >> To unsubscribe from this list: send the line "unsubscribe linux-iio" in > >> >> the body of a message to majordomo@xxxxxxxxxxxxxxx > >> >> More majordomo info at http://vger.kernel.org/majordomo-info.html > > > > -- > -- > To unsubscribe from this list: send the line "unsubscribe linux-iio" in > the body of a message to majordomo@xxxxxxxxxxxxxxx > More majordomo info at http://vger.kernel.org/majordomo-info.html -- Andreas Klinger Grabenreith 27 84508 Burgkirchen +49 8623 919966 ak@xxxxxxxxxxxxx www.it-klinger.de www.grabenreith.de -- To unsubscribe from this list: send the line "unsubscribe linux-iio" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html