Nice write-up. I generally agree with all of it, only a few comments. -- I've added only small parts but I fully agree it's bloated and many simplifications are needed. -- Logging is all around the place and needs to be standardized. -- I'll try to provide evidence on wind accuracy degradation. I still suspect roundoff errors somewhere. They disappeared in the US-units version of the driver. First we need to accept the possibility of a mistake deeper in system, outside of the driver. -- "... rainRate should not be in the driver at all. the hardware reports a value that is arbitrary, and the rainRate calculation is yet a different arbitrary calculation ..." - well, the rf packets provide a raw value, which we convert to rainfall/time period values. What do you mean by arbitrary here? I agree it should be done in a single unified way, though, to be able to provide comparable results across devices. -- We only fully process the raw radio packet data. I'm still of the opinion that just because the machine readable values come handy sometimes, we shouldn't keep it. It's sparsely reporting certain values and generally use unknown converison algorithms (even if they are accurate and correct, we have no way to tell). I'd say as part of the simplification, we should drop it entirely from the driver.
Not completely related, but is there an SDR driver with comparable functionality? I think it wouldn't be a bad idea to use one as the radio capture part and adopt the rest of the current MS driver for the decoding. On Sat, Oct 1, 2016 at 3:39 PM, mwall <[email protected]> wrote: > this is regarding the commits for meteostick driver 0.45, which included > removal of the use of weewx.units.Converter() for unit conversions and the > change to weewx.METRICWX from weewx.US for the unit system of LOOP packets > emitted by the meteostick driver. > > there have been many exchanges via email regarding the meteostick driver > between 0.37 and 0.45. luc and kobuki have done a fantastic job of reverse > engineering the davis rf protocols and the meteostick hardware and firmware > implementations. > > i'm trying to bring the exchanges back to the forum (again) so the > knowledge does not get trapped in our personal mailboxes. > > On 01 Oct 2016, at 08:43, nls wrote: > > So you mean after we agreed that using US units works better generally, > especially with wind, you reverted all > > my efforts of not using metric output back to using metric output? May I > ask, why? Because most of us are in > > Europe with metric standards? Or do you maybe think that the original > dev of WeeWx made a mistake by > > using US units? Practically everything in the packets is in US units: > wind is mph, temperature is F°, rain > > can be both. Solar values have scientifically established dimensions. > Only with the addition of the soil/leaf > > stations does the picture change somewhat. Which is admittably sparse in > relation to the rest of the instruments. > > context: > > my goal was (is) to simplify the implementation yet still ensure data > integrity. > > i consolidated units and made other changes so that i could make sense of > what is actually happening in the code. 0.44 was (and 0.45 still is) > rather clunky. > > i do not care whether the driver emits data in weewx.US or > weewx.METRICWX. (i am pretty sure that it does not matter, at least not in > the context of all the other conversions that happen, i.e., driver to > database, database to display, database to uploaders.) > > btw, kobuki, your efforts have not been reverted. > > > the facts: > > - the driver must emit units in a single unit system. that can be METRIC, > METRICWX, or US. > > - rainfall is counts > > - calculate_thermistor_temp is degree C > > - for parse_machine: > - pressure is mbar > - WT temperature is degree C directly from the device > - LMO temperature is degree C directly from the device > - wind_speed is m/s > > - for parse_raw: > - type 8 digital temperature is degree F * 10 directly from the device > sensors > - type 8 analog temperature is from calculate_thermistor_temp (degree C) > - temperatures for leaf and soil stations are from > calculate_thermistor_temp (degree C) > - pressure is mbar * 100 > - wind_speed is obtained from calc_wind_speed_ec (mph) > > > analysis: > > rain is counts, so there is a unit conversion no matter what. that is a > simple multiplication by either inches or mm or cm, depending on the unit > system. the rain_per_tip constant should have sufficient precision to > avoid inaccuracies, and the precision of python float is more than adequate > to capture rainfall amounts accurately. > > wind speed conversion is a simply multiply as well. i do not see why the > accuracy there is any different than rain counts. > > same thing for pressure. > > so no matter which unit system you choose, there will be unit > conversions. using one unit system for parse_raw and a different unit > system for parse_machine might make sense. > > but to get there i had to strip away all of the cruft so we could see what > we're working with. > > > problems with 0.44: > > - the implementation was inconsistent in its unit conversions > > - unit conversions were applied inconsistently > > - raw values and converted values were logged inconsistently (they still > are) > > - use of units.Converter obfuscates the intent. weewx.wxformulas.CtoF or > FtoC is much easier to understand. units.Converter is appropriate when the > to- and from- units are dynamic. > > - logging. it is not consistent. there is more logging around the > problem areas than there is generally. that is fine for development, but > as the driver matures the logging should mature with it. inconsistent > logging can lead to more support problems, because end-users don't see all > the messages they think they should. or because logging is inconsistent, > so logwatch and other log monitoring tools do not work as they should. > > - rainRate should not be in the driver at all. the hardware reports a > value that is arbitrary, and the rainRate calculation is yet a different > arbitrary calculation. it is more appropriate to put the rainRate > calculation into StdWXCalculate as an alternative calculation to the > standard weewx sliding window rainRate calculation. i left rainRate in the > driver for now. > > - rf tracking. the current implementation needs to be completely > rewritten. the existing code is brittle and non-obvious. i am pretty sure > that there is no reason that this driver should also be a service. > > - sensor map. this needs to be converted to output=input pattern. i have > not yet done the conversion - i did not want to combine that commit with > the disentanglement of units and logging. > > > please note that these are all fairly minor issues (other than sensor map > - that is a big deal throughout weewx that must be addressed). > > you guys have done a wonderful job so far! > > maybe the market for meteostick is so tiny that the refinements i am > looking at will never matter (the sdr usb sticks do much more than > meteostick - if only they had a pressure sensor in them!), but we still > need to make the meteostick code smell a bit less :) > > m > >
