Make gas change events always have a cylinder index

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>

core/dive.c

@@ -40,28 +40,6 @@ int event_is_gaschange(struct event *ev)
 		ev->type == SAMPLE_EVENT_GASCHANGE2;
 }
 
-/*
- * Does the gas mix data match the legacy
- * libdivecomputer event format? If so,
- * we can skip saving it, in order to maintain
- * the old save formats. We'll re-generate the
- * gas mix when loading.
- */
-int event_gasmix_redundant(struct event *ev)
-{
-	struct gasmix *mix = &ev->gas.mix;
-	int value = ev->value;
-	int o2, he;
-
-	if (value == 21)
-		return gasmix_is_air(mix);
-
-	o2 = (value & 0xffff) * 10;
-	he = (value >> 16) * 10;
-	return	o2 == get_o2(mix) &&
-		he == get_he(mix);
-}
-
 struct event *add_event(struct divecomputer *dc, unsigned int time, int type, int flags, int value, const char *name)
 {
 	int gas_index = -1;
@@ -178,7 +156,7 @@ void add_extra_data(struct divecomputer *dc, const char *key, const char *value)
 }
 
 /* this returns a pointer to static variable - so use it right away after calling */
-struct gasmix *get_gasmix_from_event(struct event *ev)
+struct gasmix *get_gasmix_from_event(struct dive *dive, struct event *ev)
 {
 	static struct gasmix dummy;
 	if (ev && event_is_gaschange(ev))
@@ -849,7 +827,7 @@ int explicit_first_cylinder(struct dive *dive, struct divecomputer *dc)
 /* this gets called when the dive mode has changed (so OC vs. CC)
  * there are two places we might have setpoints... events or in the samples
  */
-void update_setpoint_events(struct divecomputer *dc)
+void update_setpoint_events(struct dive *dive, struct divecomputer *dc)
 {
 	struct event *ev;
 	int new_setpoint = 0;
@@ -867,14 +845,14 @@ void update_setpoint_events(struct divecomputer *dc)
 		// So we make sure, this comes from a Predator or Petrel and we only remove
 		// pO2 values we would have computed anyway.
 		struct event *ev = get_next_event(dc->events, "gaschange");
-		struct gasmix *gasmix = get_gasmix_from_event(ev);
+		struct gasmix *gasmix = get_gasmix_from_event(dive, ev);
 		struct event *next = get_next_event(ev, "gaschange");
 
 		for (int i = 0; i < dc->samples; i++) {
 			struct gas_pressures pressures;
 			if (next && dc->sample[i].time.seconds >= next->time.seconds) {
 				ev = next;
-				gasmix = get_gasmix_from_event(ev);
+				gasmix = get_gasmix_from_event(dive, ev);
 				next = get_next_event(ev, "gaschange");
 			}
 			fill_pressures(&pressures, calculate_depth_to_mbar(dc->sample[i].depth.mm, dc->surface_pressure, 0), gasmix ,0, OC);
@@ -1401,6 +1379,92 @@ static void fixup_dive_pressures(struct dive *dive, struct divecomputer *dc)
 	simplify_dc_pressures(dive, dc);
 }
 
+int find_best_gasmix_match(struct gasmix *mix, cylinder_t array[], unsigned int used)
+{
+	int i;
+	int best = -1, score = INT_MAX;
+
+	for (i = 0; i < MAX_CYLINDERS; i++) {
+		const cylinder_t *match;
+		int distance;
+
+		if (used & (1 << i))
+			continue;
+		match = array + i;
+		if (cylinder_nodata(match))
+			continue;
+		distance = gasmix_distance(mix, &match->gasmix);
+		if (distance >= score)
+			continue;
+		best = i;
+		score = distance;
+	}
+	return best;
+}
+
+/*
+ * Match a gas change event against the cylinders we have
+ */
+static bool validate_gaschange(struct dive *dive, struct divecomputer *dc, struct event *event)
+{
+	int index;
+	int o2, he, value;
+
+	/* We'll get rid of the per-event gasmix, but for now sanitize it */
+	if (gasmix_is_air(&event->gas.mix))
+		event->gas.mix.o2.permille = 0;
+
+	/* Do we already have a cylinder index for this gasmix? */
+	if (event->gas.index >= 0)
+		return true;
+
+	index = find_best_gasmix_match(&event->gas.mix, dive->cylinder, 0);
+	if (index < 0)
+		return false;
+
+	/* Fix up the event to have the right information */
+	event->gas.index = index;
+	event->gas.mix = dive->cylinder[index].gasmix;
+
+	/* Convert to odd libdivecomputer format */
+	o2 = get_o2(&event->gas.mix);
+	he = get_he(&event->gas.mix);
+
+	o2 = (o2 + 5) / 10;
+	he = (he + 5) / 10;
+	value = o2 + (he << 16);
+
+	event->value = value;
+	if (he)
+		event->type = SAMPLE_EVENT_GASCHANGE2;
+
+	return true;
+}
+
+/* Clean up event, return true if event is ok, false if it should be dropped as bogus */
+static bool validate_event(struct dive *dive, struct divecomputer *dc, struct event *event)
+{
+	if (event_is_gaschange(event))
+		return validate_gaschange(dive, dc, event);
+	return true;
+}
+
+static void fixup_dc_gasswitch(struct dive *dive, struct divecomputer *dc)
+{
+	struct event **evp, *event;
+
+	evp = &dc->events;
+	while ((event = *evp) != NULL) {
+		if (validate_event(dive, dc, event)) {
+			evp = &event->next;
+			continue;
+		}
+
+		/* Delete this event and try the next one */
+		*evp = event->next;
+	}
+}
+
 static void fixup_dive_dc(struct dive *dive, struct divecomputer *dc)
 {
 	int i;
@@ -1418,6 +1482,9 @@ static void fixup_dive_dc(struct dive *dive, struct divecomputer *dc)
 	/* Fix up dive temperatures based on dive computer samples */
 	fixup_dc_temp(dive, dc);
 
+	/* Fix up gas switch events */
+	fixup_dc_gasswitch(dive, dc);
+
 	/* Fix up cylinder sensor data */
 	fixup_dc_cylinder_index(dive, dc);
 
@@ -1804,25 +1871,9 @@ extern void fill_pressures(struct gas_pressures *pressures, const double amb_pre
 
 static int find_cylinder_match(cylinder_t *cyl, cylinder_t array[], unsigned int used)
 {
-	int i;
-	int best = -1, score = INT_MAX;
-
 	if (cylinder_nodata(cyl))
 		return -1;
-	for (i = 0; i < MAX_CYLINDERS; i++) {
-		const cylinder_t *match;
-		int distance;
-
-		if (used & (1 << i))
-			continue;
-		match = array + i;
-		distance = gasmix_distance(&cyl->gasmix, &match->gasmix);
-		if (distance >= score)
-			continue;
-		best = i;
-		score = distance;
-	}
-	return best;
+	return find_best_gasmix_match(&cyl->gasmix, array, used);
 }
 
 /* Force an initial gaschange event to the (old) gas #0 */

core/dive.h

@@ -115,7 +115,6 @@ struct event {
 };
 
 extern int event_is_gaschange(struct event *ev);
-extern int event_gasmix_redundant(struct event *ev);
 
 extern int get_pressure_units(int mb, const char **units);
 extern double get_depth_units(int mm, int *frac, const char **units);
@@ -150,7 +149,7 @@ extern void fill_pressures(struct gas_pressures *pressures, const double amb_pre
 
 extern void sanitize_gasmix(struct gasmix *mix);
 extern int gasmix_distance(const struct gasmix *a, const struct gasmix *b);
-extern struct gasmix *get_gasmix_from_event(struct event *ev);
+extern int find_best_gasmix_match(struct gasmix *mix, cylinder_t array[], unsigned int used);
 
 static inline bool gasmix_is_air(const struct gasmix *gasmix)
 {
@@ -757,6 +756,7 @@ extern void update_event_name(struct dive *d, struct event* event, char *name);
 extern void add_extra_data(struct divecomputer *dc, const char *key, const char *value);
 extern void per_cylinder_mean_depth(struct dive *dive, struct divecomputer *dc, int *mean, int *duration);
 extern int get_cylinder_index(struct dive *dive, struct event *ev);
+extern struct gasmix *get_gasmix_from_event(struct dive *, struct event *ev);
 extern int nr_cylinders(struct dive *dive);
 extern int nr_weightsystems(struct dive *dive);
 
@@ -896,7 +896,7 @@ extern void set_userid(char *user_id);
 extern void set_informational_units(char *units);
 
 extern const char *get_dive_date_c_string(timestamp_t when);
-extern void update_setpoint_events(struct divecomputer *dc);
+extern void update_setpoint_events(struct dive *dive, struct divecomputer *dc);
 #ifdef __cplusplus
 }
 #endif

core/planner.c

@@ -94,12 +94,7 @@ void get_gas_at_time(struct dive *dive, struct divecomputer *dc, duration_t time
 
 int get_gasidx(struct dive *dive, struct gasmix *mix)
 {
-	int gasidx = -1;
-
-	while (++gasidx < MAX_CYLINDERS)
-		if (gasmix_distance(&dive->cylinder[gasidx].gasmix, mix) < 100)
-			return gasidx;
-	return -1;
+	return find_best_gasmix_match(mix, dive->cylinder, 0);
 }
 
 void interpolate_transition(struct dive *dive, duration_t t0, duration_t t1, depth_t d0, depth_t d1, const struct gasmix *gasmix, o2pressure_t po2)

core/profile.c

@@ -318,40 +318,23 @@ struct plot_info *analyze_plot_info(struct plot_info *pi)
  */
 int get_cylinder_index(struct dive *dive, struct event *ev)
 {
-	int i;
-	int best = 0, score = INT_MAX;
-	int target_o2, target_he;
-	struct gasmix *g;
+	int best;
+	struct gasmix *mix;
 
 	if (ev->gas.index >= 0)
 		return ev->gas.index;
 
-	g = get_gasmix_from_event(ev);
-	target_o2 = get_o2(g);
-	target_he = get_he(g);
-
 	/*
-	 * Try to find a cylinder that best matches the target gas
-	 * mix.
+	 * This should no longer happen!
+	 *
+	 * We now match up gas change events with their cylinders at dive
+	 * event fixup time.
 	 */
-	for (i = 0; i < MAX_CYLINDERS; i++) {
-		cylinder_t *cyl = dive->cylinder + i;
-		int delta_o2, delta_he, distance;
+	fprintf(stderr, "Still looking up cylinder based on gas mix in get_cylinder_index()!\n");
 
-		if (cylinder_nodata(cyl))
-			continue;
-
-		delta_o2 = get_o2(&cyl->gasmix) - target_o2;
-		delta_he = get_he(&cyl->gasmix) - target_he;
-		distance = delta_o2 * delta_o2;
-		distance += delta_he * delta_he;
-
-		if (distance >= score)
-			continue;
-		score = distance;
-		best = i;
-	}
-	return best;
+	mix = get_gasmix_from_event(dive, ev);
+	best = find_best_gasmix_match(mix, dive->cylinder, 0);
+	return best < 0 ? 0 : best;
 }
 
 struct event *get_next_event(struct event *event, const char *name)

core/save-git.c

@@ -325,7 +325,7 @@ static void save_one_event(struct membuffer *b, struct event *ev)
 		if (ev->gas.index >= 0) {
 			show_index(b, ev->gas.index, "cylinder=", "");
 			put_gasmix(b, &ev->gas.mix);
-		} else if (!event_gasmix_redundant(ev))
+		} else
 			put_gasmix(b, &ev->gas.mix);
 	}
 	put_string(b, "\n");

core/save-xml.c

@@ -269,7 +269,7 @@ static void save_one_event(struct membuffer *b, struct event *ev)
 		if (ev->gas.index >= 0) {
 			show_index(b, ev->gas.index, "cylinder='", "'");
 			put_gasmix(b, &ev->gas.mix);
-		} else if (!event_gasmix_redundant(ev))
+		} else
 			put_gasmix(b, &ev->gas.mix);
 	}
 	put_format(b, " />\n");

desktop-widgets/maintab.cpp

@@ -946,7 +946,7 @@ void MainTab::acceptChanges()
 					get_dive_dc(mydive, dc_number)->divemode = displayed_dc->divemode;
 				}
 			);
-			MODIFY_SELECTED_DIVES(update_setpoint_events(get_dive_dc(mydive, dc_number)));
+			MODIFY_SELECTED_DIVES(update_setpoint_events(mydive, get_dive_dc(mydive, dc_number)));
 			do_replot = true;
 		}
 		if (displayed_dive.watertemp.mkelvin != cd->watertemp.mkelvin)
@@ -1237,7 +1237,7 @@ void MainTab::divetype_Changed(int index)
 		return;
 	struct divecomputer *displayed_dc = get_dive_dc(&displayed_dive, dc_number);
 	displayed_dc->divemode = (enum dive_comp_type) index;
-	update_setpoint_events(displayed_dc);
+	update_setpoint_events(&displayed_dive, displayed_dc);
 	markChangedWidget(ui.DiveType);
 	MainWindow::instance()->graphics()->recalcCeiling();
 }