In the planner if one adds two or more cylinders with the same gasmix
(e.g. back gas and bottom stage 18/45) the drop down and data in the
used gas column of the planner points table will be filled with a more
verbose string mentioning also the cyl number and the cyl type
description.
Makes it easier in such a case to select the right cylinder.
Introduces also a helper function which tells you if there is another
cylinder with the same gasmix as the provided cylinder.
This also has an option if it should consider unused cylinders or not.
Signed-off-by: Stefan Fuchs <sfuchs@gmx.de>
When planning a new dive (not replan!!!) based on an existing (planned)
dive, the cylinders from the existing selected dive are copied.
This patch guarantees that cylinders which had been marked as "unused"
are indeed copied as well. Sounds strange at the first moment but makes
sense because if one marks a cylinder explicitly as "unused" in the
planner instead of deleting it that does mean that one wants to keep
this cylinder to have it available and be able to reenable it later-on.
Signed-off-by: Stefan Fuchs <sfuchs@gmx.de>
When we merge dives, the sample_start and sample_end pressures are only
used in-memory for displaying data to the user. However, we should
update them as well as this will show the user the correct data in the
equipment/cylinder and i.e. SAC calculation.
Fixes#577
Signed-off-by: Miika Turkia <miika.turkia@gmail.com>
As these are probably manually entered dives with incomplete data, it is
better not to merge them.
See #561
Signed-off-by: Miika Turkia <miika.turkia@gmail.com>
The momentary SAC rate got broken by the multiple ressure handling too,
and always used just the first cylinder.
This uses the new "get_gasmix()" helper to see what you're breathing,
and will do the SAC rate over all the cylinders that contain that gas.
So it should now DTRT even for sidemount diving (assuming you had the
same gas in the sidemount cylinders).
NOTE! We could just do the SAC rate over *all* the gases you have
pressures for, and maybe that's the right thing to do. The ones you are
not breating from shouldn't have their pressure change. But maybe some
people add their drysuit argon gas to the gas list?
So this may need more work, but it's a step in the right direction.
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
Signed-off-by: Dirk Hohndel <dirk@hohndel.org>
This finally handles multiple cylinder pressures, both overlapping and
consecutive, and it seems to work on the nasty cases I've thrown at it.
Want to just track five different cylinders all at once, without any
pesky gas switch events? Sure, you can do that. It will show five
different gas pressures for your five cylinders, and they will go down
as you breathe down the cylinders.
I obviously don't have any real data for that case, but I do have a test
file with five actual cylinders that all have samples over the whole
course of the dive. The end result looks messy as hell, but what did
you expect?
HOWEVER.
The only way to do this sanely was
- 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.
- the "populate_pressure_information()" function had to be rewritten
quite a bit. The good news is that it's actually simpler now, although
I would not go so far as to really call it simple. It's still
complicated and suble, but now it explicitly just does one cylinder at
a time.
It *used* to have this insanely complicated "keep track of the pressure
ranges for every cylinder at once". I just couldn't stand that model
and keep my sanity, so it now just tracks one cylinder at a time, and
doesn't have an array of live data, instead the caller will just call
it for each cylinder.
- get rid of some of our hackier stuff, like the code that populates the
plot_info data code with the currently selected cylinder number, and
clears out any other pressures. That obviously does *not* work when you
may not have a single primary cylinder any more.
Now, the above sounds like all good things. Yeah, it mostly is.
BUT.
There's a few big downsides from the above:
- there's no sane way to do this as a series of small changes.
The change to make the plot_info take an array of cylinder pressures
rather than the sensor+pressure model really isn't amenable to "fix up
one use at a time". When you switch over to the new data structure
model, you have to switch over to the new way of populating the
pressure ranges. The two just go hand in hand.
- 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.
- I have tested it, and I think the code is better, but hey, it's a
fairly large patch to some of the more complex code in our code base.
That "interpolate missing pressure fields" code really isn't pretty. It
may be prettier, but..
Anyway, without further ado, here's the patch. No sign-off yet, because I
do think people should look and comment. But I think the patch is fine,
and I'll fix anythign that anybody can find, *except* for that TankItem
thing that I will refuse to touch. That class is ugly. It needs to have
access to the actual dive.
Note how it actually does remove more lines than it adds, and that's
despite added comments etc. The code really is simpler, but there may be
cases in there that need more work.
Known missing pieces that don't currently take advantage of concurrent
cylinder pressure data:
- the momentary SAC rate coloring for dives will need more work
- dive merging (but we expect to generally normally not merge dive
computers, which is the main source of sensor data)
- actually taking advantage of different sensor data from different
dive computers
But most of all: Testing. Lots and lots of testing to find all the
corner cases.
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
Signed-off-by: Dirk Hohndel <dirk@hohndel.org>
We had a "add_sample_pressure()" helper functions that was local to just
the libdivecomputer downloading code, but it really is applicable to
pretty much any code that adds cylinder pressure data to a sample.
Also add another helper: "legacy_format_o2pressures()" which checks the
sample data to see if we can use the legacy format, and returns the o2
pressure sensor to use for that legacy format.
Because both the XML and the git save format will need a way to save the
compatible old-style information, when possible, but save an extended
format for when we have data from multiple concurrent sensors.
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
Signed-off-by: Dirk Hohndel <dirk@hohndel.org>
This tries to sanely handle the case of a dive computer reporting
multiple cylinder pressures concurrently.
NOTE! There are various "interesting" situations that this whole issue
brings up:
- some dive computers may report more cylinder pressures than we have
slots for.
Currently we will drop such pressures on the floor if they come for
the same sample, but if they end up being spread across multiple
samples we will end up re-using the slots with different sensor
indexes.
That kind of slot re-use may or may not end up confusing other
subsurface logic - for example, make things believe there was a
cylidner change event.
- some dive computers might send only one sample at a time, but switch
*which* sample they send on a gas switch event. If they also report
the correct sensor number, we'll now start reporting that pressure in
the second slot.
This should all be fine, and is the RightThing(tm) to do, but is
different from what we used to do when we only ever used a single
slot.
- When people actually use multiple sensors, our old save format will
start to need fixing. Right now our save format comes from the CCR
model where the second sensor was always the Oxygen sensor.
We save that pressure fine (except we save it as "o2pressure" - just
an odd historical naming artifact), but we do *not* save the actual
sensor index, because in our traditional format that was always
implicit in the data ("it's the oxygen cylinder").
so while this code hopefully makes our libdivecomputer download do the
right thing, there *will* be further fallout from having multiple
cylinder pressure sensors. We're not done yet.
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
Signed-off-by: Dirk Hohndel <dirk@hohndel.org>
This is a very timid start at making us actually use multiple sensors
without the magical special case for just CCR oxygen tracking.
It mainly does:
- turn the "sample->sensor" index into an array of two indexes, to
match the pressures themselves.
- get rid of dive->{oxygen_cylinder_index,diluent_cylinder_index},
since a CCR dive should now simply set the sample->sensor[] indices
correctly instead.
- in a couple of places, start actually looping over the sensors rather
than special-case the O2 case (although often the small "loops" are
just unrolled, since it's just two cases.
but in many cases we still end up only covering the zero sensor case,
because the CCR O2 sensor code coverage was fairly limited.
It's entirely possible (even likely) that this migth break some existing
case: it tries to be a fairly direct ("stupid") translation of the old
code, but unlike the preparatory patch this does actually does change
some semantics.
For example, right now the git loader code assumes that if the git save
data contains a o2pressure entry, it just hardcodes the O2 sensor index
to 1.
In fact, one issue is going to simply be that our file formats do not
have that multiple sensor format, but instead had very clearly encoded
things as being the CCR O2 pressure sensor.
But this is hopefully close to usable, and I will need feedback (and
maybe test cases) from people who have existing CCR dives with pressure
data.
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
Signed-off-by: Dirk Hohndel <dirk@hohndel.org>
We currently carry two pressures around for all the samples and plot
info, but the second pressure is reserved for CCR dives as the O2
cylinder pressure.
That's kind of annoying when we *could* use it for regular sidemount
dives as the secondary pressure.
So start prepping for that instead: don't make it "pressure" and
"o2pressure", make it just be an array of two pressure values.
NOTE! This is purely mindless prepwork. It literally just does a
search-and-replace, keeping the exact same semantics, so "pressure[1]"
is still just O2 pressure.
But at some future date, we can now start using it for a second sensor
value for sidemount instead.
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
Signed-off-by: Dirk Hohndel <dirk@hohndel.org>
The core to avoid adding redundant gas switch events was completely
buggered, and caused the result list to be corrupted if it ever
triggered. This should fix it.
Fixes: b5de08b7 ("No gas change event on merging dives with same gas")
Reported-by: Jan Mulder <jlmulder@xs4all.nl>
Cc: Miika Turkia <miika.turkia@gmail.com>
Cc: Dirk Hohndel <dirk@hohndel.org>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
Signed-off-by: Dirk Hohndel <dirk@hohndel.org>
... for consistency, while we are at it.
There are still some internal depth variables which are ints
somebody might take a go at those.
Signed-off-by: Robert C. Helling <helling@atdotde.de>
This is needed in the altitude pressure conversion as there
negative altitudes are possible (for diving in the netherlands
or the Dead Sea).
Signed-off-by: Robert C. Helling <helling@atdotde.de>
Using gcc option "-Wfloat-conversion" is useful to catch
potential conversion errors (where lrint should be used).
rint returns double and still raises the same warning,
this is why this change updates all rint calls to lrint.
In few places, where input type is a float, corresponding
lrinf is used.
Signed-off-by: Jeremie Guichard <djebrest@gmail.com>
When merging, we should treat an empty dive site (which will be deleted
on save) the same as not having a dive site.
Signed-off-by: Dirk Hohndel <dirk@hohndel.org>
I have no idea why we only merged air temperatures. But it was very
explicit (even the function doing the merging was named
"merge_airtemp()"), and water temperatures were left alone.
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
The test for the dive being a planned dive was completely bogus:
- it should use "same_string()" which correctly checks for NULL
- the string it checks for is obviously spelled wrong anyway.
Reported-by: Alessandro Volpi <volpial@gmail.com>
Fixes: a031dbbbd ("When merging planned dives keep all cylinders")
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
This makes some further updates to the new cylinder merging code:
- avoid re-using the cylinder if the usage type (OC/diluent/O2) is
different between the two dives, even if the gasmix might be the
same.
- avoid re-using a cylinder if the user has manually added pressure
data for it (and the pressures don't match)
- when deciding to reuse a cylinder, make sure that we merge as much of
the type information as makes sense.
This will potentially result in more cylinders that might need manual
cleanup, but at least we won't be throwing out user data. And in most
cases where merging happens, none of this is an issue (because the data
comes fresh from a dive computer, and won't have been manually edited to
trigger the new rules).
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
The old cylinder merging code depended on the preferred dive having all
the cylinders, and the newly merged dive was just forced to pick from
that existing set of cylinders.
That worked ok if you have a "main" dive computer that you have all the
gases programmed for, and you download that first, and then you download
any secondary data later.
But it completely messed up if the second dive computer had gases that
the first one didn't know about, and just basically ended up doing
random things.
This rewrites the whole thing to actually try to create a union of the
two sets of cylinders when merging, with sane matching so that if the
cylinders match you won't get duplicates.
Miika Turkia hit this when he only used one gas, but had several gases
defined in his OSTC that he downloaded after his Vyper (with had just
the single gas defined).
This should fix that case (at least it does for my xml merging test-case
that showed the same problem after some munging).
Reported-by: Miika Turkia <miika.turkia@gmail.com>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
There is no need to have two variables for the same purpose.
[Dirk Hohndel: changed to keep the two separate functions as otherwise
we no longer parse existing repos successfully]
Signed-off-by: Joakim Bygdell <j.bygdell@gmail.com>
Signed-off-by: Dirk Hohndel <dirk@hohndel.org>
In order to streamline the view between desktop and mobile we need to save
selected profile related settings to git.
Signed-off-by: Joakim Bygdell <j.bygdell@gmail.com>
When the first leg in the planner is not cylinder 0, a gaschange
event at t=1s is inserted. In the profile, we should treat that
as inital gas, so no pressure information is printed for cylinder 0
that is used nominally for one second.
This fixes a problem reported by Willem.
Signed-off-by: Robert C. Helling <helling@atdotde.de>
When merging a real dive with a planned dive (for comparison),
we should not try to be clever in merging similar cylinders,
rather keep the union of both cylinder sets as the two versions
of the dive might differ in exctly which gas and how much of it
was used.
Increase MAX_CYLINDERS to 20 to make room for this. We warn if we
exceed this number.
[Dirk Hohndel: I had mistakenly pushed out an earlier version of this
commit, so this fixes things up to the final version]
Signed-off-by: Robert C. Helling <helling@atdotde.de>
Signed-off-by: Dirk Hohndel <dirk@hohndel.org>
When merging a real dive with a planned dive (for comparison),
we should not try to be clever in merging similar cylinders,
rather keep the union of both cylinder sets as the two versions
of the dive might differ in exctly which gas and how much of it
was used.
Increase MAX_CYLINDERS to 20 to make room for this.
Signed-off-by: Robert C. Helling <helling@atdotde.de>
In the cylinder table, the last column ("use") always showed
OC-GAS. Editing was enabled, but the user had to guess to enter
a small integer meaning dilluent or CCR oxygen cylingder. I guess,
nobody has ever done that.
This patch makes this column clickable. A click toggles if the cylinder
is used for planning or not. This wait it is much easier to investigate
the consequences of gas loss on a plan.
Signed-off-by: Robert C. Helling <helling@atdotde.de>
This adds tooltips for the equipment tab for each cylinder, showing the
amount of gas used.
When you mouse over the size and working pressure fields, the tooltip will
show the amount of gas used (along with start and end gas volumes). And
when you mouse over the start and end pressures, it will show the start
and end gas volumes, and the Z factor used.
I started doing this because of the gas volume questions in the last day
or two (and a few from a few weeks ago). When even Robert Helling starts
wondering about the effects of compressibility on the SAC calculation, our
numbers are clearly too opaque.
With these tooltips, at least you can see what went into the used gas
calculations, instead of having to add debugging options to print out Z
factors.
[ This patch also adds a "rint()" to get the rounding right in the
gas_volume() function. Although rounding to the nearst milliliter
really doesn't matter, it's the right thing to do after doing FP
calculations ;^]
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
When merging dives, this will skip the gas change event if both dives
use same gas.
Fixes#1099
Signed-off-by: Miika Turkia <miika.turkia@gmail.com>
Signed-off-by: Dirk Hohndel <dirk@hohndel.org>
This removes 'fixup_dc_cylinder_index()', which was added to fix up the
pressure sensor indexes from the Atomic Cobalt dive computer.
Even for the Cobalt it really shouldn't matter, because the
libdivecomputer backend for the Cobalt actually tries to do the right
thing. See for example commit 8853a1ccd422 ("Associate the pressure
samples with the primary tank.") in libdivecomputer.
Some historical digging shows that the subsurface sample pressure index
code came in from commit e32ba4d6d8 ("Improve tank handling for
Cobalt"), dated Tue Oct 28 13:48:15 2014.
And the libdivecomputer "use the right cylinder" code was around the
same time (Fri Oct 10 20:29:17 2014 +0200).
So I suspect that subsurface needed the fixup based on an older version
of libdivecomputer. Jef's patch is a couple of weeks before, but we may
not have tracked libdivecomputer religiously.
The reason to remove this code is because it can (and does) mess up the
sensor index when it is actually reliable, like in the multi-sensor case
of the Suunto EON Steel.
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
Signed-off-by: Dirk Hohndel <dirk@hohndel.org>
So, prefs.save_userid_local is being set outside of
a preferences set (it's set to true and false while
loading the files via xml or git) and because of that
I had to bypass a few method calls.
When something triggers a preferences change, the
application will be notified that the preferences
changed, thing that I couldn't do while reading the
xml or git because that should be local-only.
Signed-off-by: Tomaz Canabrava <tomaz.canabrava@gmail.com>
Signed-off-by: Dirk Hohndel <dirk@hohndel.org>
The cylinder use field was not merged when dives were merged. This is
normally not noticeable, since hopefully the cylinder use should be the
same anyway, but when re-downloading the dives from the EON Steel after
updating it to also get cylinder use data, the dive merging threw the
data away again since the original dive lacked it.
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
Signed-off-by: Dirk Hohndel <dirk@hohndel.org>
Our primary dive computer really is special, not just because it's the
first one: it's directly embedded in the "struct dive", and so if you
just walk the divecomputer list, you'll miss it, because it's not _on_
the list, it is the very head _of_ the list.
We had that bug in copy_dive(), and it turns out we have it in
clear_dive() too: clear_dive() would free all the dive computers on the
list, but not the actual primary one.
This is a minor memory leak, no more, so it's not exactly critial, but
let's just do it right.
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
Signed-off-by: Dirk Hohndel <dirk@hohndel.org>
The previous patch (Planner: add best mix EAD preference) used the term EAD
(equivalent air depth) in variable names and strings, when it should have been
END (equivalent narcotic depth).
They're not the same thing and shouldn't be confused.
Signed-off-by: Rick Walsh <rickmwalsh@gmail.com>
Signed-off-by: Dirk Hohndel <dirk@hohndel.org>
Add best mix EAD preference and UI, along with a tooltip describing what it
does
Signed-off-by: Rick Walsh <rickmwalsh@gmail.com>
Signed-off-by: Dirk Hohndel <dirk@hohndel.org>
Best mix O2 calculated based on planner Bottom O2 preference
Best mix He calculated based on EAD of 30m (should be made user-configurable)
Signed-off-by: Rick Walsh <rickmwalsh@gmail.com>
Signed-off-by: Dirk Hohndel <dirk@hohndel.org>
John Van Ostrand reports that when he dives using two cylinders using
sidemounts, the per-cylinder SAC rate display is very misleading.
What happens is that since the two cylinders are used together (but
without a manifold), John is alternating between the two but not
actually adding gas switches in the profile. As a result, the profile
looks like only one cylinder is used, even though clearly the other
cylinder gets breathed down too.
The per-cylinder SAC rate calculations would entirely ignore the
cylinder that didn't have gas switch events to it, and looking at the
info window it would look like John had a truly exceptional SAC rate.
But then in the general statistics panel that actually takes the whole
gas use into account, the very different real SAC rate would show up.
The basic issue is that if we don't have full use information for the
different cylinders, we would account the whole dive to just a partial
set. We did have a special case for this, but that special case only
really worked if the first cylinder truly was the only cylinder used.
This patch makes us see the difference between "only one cylinder was
used, and I can use the overall mean depth for it" and "more than one
cylinder was used, but I don't know what the mean depths might be".
Reported-by: John Van Ostrand <john@vanostrand.com>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
Signed-off-by: Dirk Hohndel <dirk@hohndel.org>
Now that gas switch events always have indices into the cylinder table,
start using that to look up the gas mix from the cylinders rather than
from the gas switch event itself. In other words, the cylinder index is
now the primary data for gas switch events.
This means that now as you change the cylinder information, the gas
switch events will automatically update to reflect those changes.
Note that on loading data from the outside (either from a xml file, from
a git/cloud account, or from a dive computer), we may or may not
initially have an index for the gas change event. The external data may
be from an older version of subsurface, or it may be from a
libdivecomputer download that just doesn't give index data at all.
In that case, we will do:
- if there is no index, but there is explicit gas mix information, we
will look up the index based on that gas mix, picking the cylinder
that has the closest mix.
- if there isn't even explicit gas mix data, so we only have the event
value from libdivecomputer, we will turn that value into a gasmix,
and use that to look up the cylinder index as above.
- if no valid cylinder information is available at all, gas switch
events will just be dropped.
When saving the data, we now always save the cylinder index, and the gas
mix associated with that cylinder (that gas mix will be ignored on load,
since the index is the primary, but it makes the event much easier to
read).
It is worth noting we do not modify the libdivecomputer value, even if
the gasmix has changed, so that remains as a record of the original
download.
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
Signed-off-by: Dirk Hohndel <dirk@hohndel.org>
In commit df4e26c875 ("Start sanitizing gaschange event information")
back about a year and a half ago, I started sanitizing the gas switch
event data, allowing gas switches to be associated with a particular
cylinder index rather than just the gas mix that is switched to.
But that initial step only _allowed_ a gas switch event to be associated
with a particular cylinder, the primary model was still to just specify
the mix.
This finally takes the next step, and *always* associates a gas switch
event with a particular cylinder. Instead of then looking up the
cylinder by trying to match gas mixes at runtime, subsurface now looks
it up when loading the dive initially as part of the dive fixup code.
The switch event still has an a separate gas mix associated with it, but
this patch also starts preparing for entirely relying on the gas mix in
the cylinder itself, by starting to pass in not just the event but also
the dive pointer to the routines that look up gas mix details.
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
Signed-off-by: Dirk Hohndel <dirk@hohndel.org>
The gas switch event handling is somewhat fragile, mostly because the
legacy event encoding for gas switches is odd. It's also limited to
whole percentages, unlike our internal gas mix model.
In addition, it also ends up comparing the values to the raw permille
values, which is wrong for air, and wouldn't match our O2_IN_AIR which
is 209 permille (closest approximation to 20.946%).
So handle air separately, since "21" really is a valid oxygen value for
air, and should match 20.9%. And use the proper accessor functions to
get the gasmix values.
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
Signed-off-by: Dirk Hohndel <dirk@hohndel.org>
