On 11 May, 2017 - Jef Driesen wrote: > On 2017-05-10 15:43, Anton Lundin wrote: > >>>That said, I think setpoint values are still interesting to see, to > >>>validate how well the controller did manage to try to keep the o2 close > >>>to the setpoint. > >> > >>After looking at the data, I had the impression that the setpoint > >>value is "unused" because it seems to just contain some "dummy" > >>value (for example the last used value, or some default value). > >> > >>I'll illustrate with an example dive from Steve's Petrel (*). This > >>dive has a fixed setpoint of 0.70 on every sample, but the ppo2 > >>values range from 0.32 to 1.74! > > > >This sounds like a mCCR. Then its up to the diver to press a button > >until the ppo2 matches what the diver would like to have. > > That makes sense! > > >>(*) I can send you the data if you want to take a look. I don't know > >>if Steve is okay with sending his dives to a public mailinglist, so > >>I didn't attach it to this email. > >> > >>To me that doesn't look like the dive computer is even trying to > >>keep the ppo2 close to the setpoint. At least not to the setpoint > >>value that's stored in the sample. Hence my question whether this > >>value is relevant or not? > > > >I think so. > > > >My guess is that the setpoint is what the computer will continue to use > >as its ppo2 value if it looses the connection with the sensors. > > > >Its better to expose the information to the user, and let the user > >ignore/delete it if they don't care about it. > > My only concern here is that if the info is useless, like those zero > millivolt values when external O2 sensor monitoring is disabled, the > we better don't report them at all. But if that's not the case, then > it's of course fine to keep reporting them. > > >>>The only real comment about the code is that I would have liked to see > >>>the calibration factor kept as a int, and just change the unit factor > >>>from .00001 to .000022, between the models. > >> > >>What would be the advantage of that? That would mean yet some other > >>field to store the scaling factor, or doing some "if (model == > >>PREDATOR)" when calculating the ppo2. Now it's just done once in > >>advance, making the conversion from millivolt to ppO2 independent of > >>the model. I'm even tempted to pre-multiply the value with the > >>100000.0 factor too, to get rid of an extra > > > >I'd store it as a separate calibration factor unit. Anyway, if you > >multiply the calibration factor with 2.2 as of now, its better to > >include the 1/100000 factor to, rather than having them in two separate > >places. > > I replaced your patch with your latest version, and updated the rest > of the series. Please have a look. If you are okay with the changes, > I'll merge them to master.
LGTM. You can add my Reviewed-by: Anton Lundin <gla...@acc.umu.se> tag to the patches if you like. I think its a good idea to keep the O2 sensors behind the SENSOR_AVERAGE ifdef for now, and try to figure out a nice scheme to expose both the raw sensors and the "voted average" value as different things to the application. The two other libdivecomputer backends emitting DC_SAMPLE_PPO2 are diverite_nitekq_parser and the hw_ostc_parser. The HW values are raw sensor values, but do you know what the diverite_nitekq_parser value are? //Anton -- Anton Lundin +46702-161604 _______________________________________________ subsurface mailing list subsurface@subsurface-divelog.org http://lists.subsurface-divelog.org/cgi-bin/mailman/listinfo/subsurface