Real support for multiple concurrent sensors..

[Robert Helling]

Hi,

a few quick comments from my side (without looking at the actual code):

> On 27. Jul 2017, at 06:57, Linus Torvalds <torvalds at linux-foundation.org> wrote:
> 
> - actually make the "struct plot_info" have all the cylinder pressures
>   (so no "sensor index and pressure" - every cylinder has a pressure for
>   every plot info entry)
> 
>   This obviously makes the plot_info much bigger. We used to have
>   MAX_CYLINDERS be a fairly generous 8, which seems sane. The planning
>   code made that 8 be 20. That seems questionable. But whatever.
> 
>   The good news is that the plot-info should hopefully get freed, and
>   only be allocated one dive at a time, so the fact that it is big and
>   nasty shouldn't be a scaling issue, though.
> 

What do people think, how much of a problem is this? Even though the planner is the main user of the increase in MAX_CYLINDERS, strictly speaking it’s not really planner specific: We increased it so we can have different versions of a dive (with different cylinders) as different “dive computers” and the original dive that was problematic was a dive where one computer held the planned version of the dive and the other the actual executed version of the dive (and the fill was not perfect so the planned mixes differs slightly from the real mixes). So, when merging the two versions into one dive, we increased MAX_CYLINDERS not to lose any of those even for a multi-stage dive.

That said, the increased number only came from having different dive computers. So We could get away with the old 8 if instead of the current dive—>cylinders[] we made it part of the dive computer, i.e. dive->dc.cylinders[]. But I guess this refactoring would involve some major surgery (and for some reason, QtCreator’s “Find usage” reports only a fraction of uses) but already

th-ws-tmpim01:subsurface helling$ git grep '>cylinder\['|wc
     205    1035   18274

But this might be worthwhile to do for aesthetic reasons anyway. What do people think?

> 
> - Some of our code *depended* on the "sensor+pressure" model. I fixed all
>   the ones I could sanely fix. There was one particular case that I just
>   couldn't sanely fix, and I didn't care enough about it to do something
>   insane.
> 
>   So the only _known_ breakage is the "TankItem" profile widget. That's
>   the bar at the bottom of the profile that shows which cylinder is in
>   use right now. You'd think that would be trivial to fix up, and yes it
>   would be - I could just use the regular model of
> 
>     firstcyl = explicit_first_cylinder(dive, dc)
>     .. then iterate over the gas change events to see the others ..
> 
>   but the problem with the "TankItem" widget is that it does its own
>   model, and it has thrown away the dive and the dive computer
>   information. It just doesn't even know. It only knows what cylinders
>   there are, and the plot_info. And it just used to look at the sensor
>   number in the plot_info, and be done with that. That number no longer
>   exists.

As I said, I haven’t looked at the code. But from what I would expect to see as the user would be that the TankItem for a given moment in time should report the gas whose pressure has most recently changed (i.e. that was connected to the divers mouth). As far as I can see, the only reason why actually more than one gas is used is that it’s a CCR dive (where the TankItem should be disabled anyway) but maybe that assumption is incorrect. Do we have non-CCR dives from the wild where actually more than one gas changing pressure (beyond sensor jitter or tanks cooling down) at any point in time? Maybe another approach would be to guess gaschange events from the sensor data?

Best
Robert

--
.oOo.oOo.oOo.oOo.oOo.oOo.oOo.oOo.oOo.oOo.oOo.oOo.oOo.oOo.oOo.oOo.oOo.oOo.oOo.oO
Robert C. Helling     Elite Master Course Theoretical and Mathematical Physics
                      Scientific Coordinator
                      Ludwig Maximilians Universitaet Muenchen, Dept. Physik
                      Phone: +49 89 2180-4523  Theresienstr. 39, rm. B339
                      http://www.atdotde.de

Enhance your privacy, use cryptography! My PGP keys have fingerprints
A9D1 A01D 13A5 31FA 6515  BB44 0820 367C 36BC 0C1D    and
DCED 37B6 251C 7861 270D  5613 95C7 9D32 9A8D 9B8F





-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.subsurface-divelog.org/pipermail/subsurface/attachments/20170727/3deaa895/attachment.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 495 bytes
Desc: Message signed with OpenPGP
URL: <http://lists.subsurface-divelog.org/pipermail/subsurface/attachments/20170727/3deaa895/attachment.sig>