On Fri, Jan 2, 2015 at 8:43 PM, Dirk Hohndel <[email protected]> wrote:
>
> I reverted the temperature part and just pushed that out.
Ok, thanks, that fixes it.
And while you did that, I went ahead and tried to analyze a bit deeper
what exactly was going on. Because it turns out that the XML save
format was to be impervious to this problem.
And that, in turn, is because it turns out that the git loading and
the XML loading are subtly different.
With the git save format, on sample loading, we always start with the
previous sample state, except for cylinder pressure which we clear:
* By default the sample data does not change unless the
* save-file gives an explicit new value. So we copy the
* data from the previous sample if one exists, and then
* the parsing will update it as necessary.
*
* There are a few exceptions, like the sample pressure:
* missing sample pressure doesn't mean "same as last
* time", but "interpolate". We clear those ones
* explicitly.
and the code literally does:
if (sample != dc->sample) {
memcpy(sample, sample-1, sizeof(struct sample));
sample->cylinderpressure.mbar = 0;
}
which matches the comment there.
HOWEVER. The XML parser is different, and rather than the above fairly
obvious code ("copy the whole old sample, then clear the cylinder
pressure") it does:
static void sample_start(void)
{
cur_sample = prepare_sample(get_dc());
cur_sample->ndl.seconds = lastndl;
cur_sample->in_deco = lastindeco;
cur_sample->stoptime.seconds = laststoptime;
cur_sample->stopdepth.mm = laststopdepth;
cur_sample->cns = lastcns;
cur_sample->setpoint.mbar = lastpo2;
cur_sample->sensor = lastsensor;
}
static void sample_end(void)
{
if (!cur_dive)
return;
finish_sample(get_dc());
lastndl = cur_sample->ndl.seconds;
lastindeco = cur_sample->in_deco;
laststoptime = cur_sample->stoptime.seconds;
laststopdepth = cur_sample->stopdepth.mm;
lastcns = cur_sample->cns;
lastpo2 = cur_sample->setpoint.mbar;
cur_sample = NULL;
}
so what happened was that since I use the git save format, any missing
temperatures in my save files would get populated as "same as the old
sample". Then we cleared duplicates, so when we saved, we saved the
same thing.
And then commit 6cf3787a0ed1 removed the clearing, so it would save
that changed state with all the duplicate temperatures.
But if you used the XML save format, any missing temperatures would
never have been initialized in the first place, so that commit made no
difference.
Anyway, I'd suggest applying the attached patch that makes the git
save format not save redundant temperatures. That actually makes your
partial revert be a non-issue.
I *also* wonder whether we should perhaps try to make the git and xml
saving logic be closer to each other. I think the git logic is perhaps
better in that it's simpler and doesn't special-case a lot of
different sample cases.
But that is a separate issue.
Hmm?
I'll take a look at the odd cylinder switch thing next.
Linus
From 927c7880f5315a10953acc095ed4709800f54a8f Mon Sep 17 00:00:00 2001
From: Linus Torvalds <[email protected]>
Date: Fri, 2 Jan 2015 21:29:40 -0800
Subject: [PATCH] git save format: Don't save redunant temperature sample data
The git sample loader will fill in temperature data from the previous
entry anyway, so saving repeated temperatures is just wasteful.
It turns out that commit 6cf3787a0ed1 ("Remove code that zeroes out
duplicate oxygen sensor and temperature values") removed the explicit
redundant temperature removal in the dive fixup, which had hidden this
issue.
Cc: willem ferguson <[email protected]>
Signed-off-by: Linus Torvalds <[email protected]>
---
save-xml.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/save-xml.c b/save-xml.c
index 7317446f72f2..29e09dc98cfc 100644
--- a/save-xml.c
+++ b/save-xml.c
@@ -213,7 +213,10 @@ static void save_sample(struct membuffer *b, struct sample *sample, struct sampl
{
put_format(b, " <sample time='%u:%02u min'", FRACTION(sample->time.seconds, 60));
put_milli(b, " depth='", sample->depth.mm, " m'");
- put_temperature(b, sample->temperature, " temp='", " C'");
+ if (sample->temperature.mkelvin && sample->temperature.mkelvin != old->temperature.mkelvin) {
+ put_temperature(b, sample->temperature, " temp='", " C'");
+ old->temperature = sample->temperature;
+ }
put_pressure(b, sample->cylinderpressure, " pressure='", " bar'");
put_pressure(b, sample->o2cylinderpressure, " o2pressure='", " bar'");
--
2.2.1.212.gc5b9256
_______________________________________________
subsurface mailing list
[email protected]
http://lists.subsurface-divelog.org/cgi-bin/mailman/listinfo/subsurface
