request for comments
Dirk Hohndel
dirk at hohndel.org
Tue Aug 21 22:38:24 PDT 2012
Linus (and everyone else),
Here's a start at implementing the trips
I don't think this should be pulled (for one thing it contains tons of
un-ifdef-ed printf calls). But I would like to know what you (and
others) think about the design decisions that I made here so far, before
I go any further in the implementation.
So with this the functionally is equivalent to what we have (I think)
but it now saves (and loads) the explicit trip data structures to/from
the XML file and maintains them as a GList in memory.
What I really would like feedback on is
- the on disk format
I am dropping the duration and am adding a "tripentry='TRUE'" property
to the dive tag (which I then ignore when reading - how's that for
consistency). I am keeping the negative trip numbers to make it easier
to search in the XML files
e.g., for a trip with four dives in Hoodsport:
<dive number='4' tripid='-21' date='2010-12-04' time='10:38:00' tripentry='TRUE'>
<location>Sund Rock, Hoodsport, WA, USA</location>
</dive>
- the in memory data structure
I have a GList that has all the trips in it and use tripid entries to
connect those and dives. The GList has ->data links to what are actually
struct dive elements. This is a bit wasteful in memory use, but is super
nice for passing them around and saving and storing them; but it does
seem a little artificial. Yet the changes needed in other pieces of the
code seemed to make this far desirable over introducing a completely new
data structure for the trips
- the logic that I use to match the trips with dives (and create new
trips when there are none that match). This code has not been heavily
tested and I bet there are corner cases I still get wrong. If you find
any, please let me know.
So far the code doesn't allow you to edit or in any way manipulate trips
- that I will implement once this part of the code passes muster.
What I did try was to manually edit an XML file so it has dives at top
level (those are marked with a magic tripid of -1). This seems to all
work as expected (but visually I'm a bit unhappy as they blend into a
dive trip "above" them once that is expanded - so I need a better way to
separate them in an expanded divelist, I guess - maybe some modification
to the dive number column (marked '#') where for dives that are part of
trips the trip number gets repeated or something).
Anyway, please let me know what you think.
/D
The following changes since commit 5726a50d89d1f76b6bc2cfb96568d41d27f2b63e:
Improve group selection semantics (2012-08-20 06:45:56 -0700)
are available in the git repository at:
git://git.hohndel.org/subsurface.git trips
for you to fetch changes up to d19f88fade1942c3dcbbfe3bec681abc28970b09:
First cut of explicit trip tracking (2012-08-21 22:10:46 -0700)
----------------------------------------------------------------
Dirk Hohndel (1):
First cut of explicit trip tracking
dive.c | 5 ++++
dive.h | 6 ++++
divelist.c | 176 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-------------------------
parse-xml.c | 12 +++++++-
save-xml.c | 23 ++++++++++++--
5 files changed, 178 insertions(+), 44 deletions(-)
for those who prefer a straight diff, here it is:
diff --git a/dive.c b/dive.c
index aee09d5..9fdc8b9 100644
--- a/dive.c
+++ b/dive.c
@@ -706,6 +706,11 @@ struct dive *try_to_merge(struct dive *a, struct dive *b)
if ((a->when >= b->when + 60) || (a->when <= b->when - 60))
return NULL;
+ /* summary dive_trip entries should never be considered for merging;
+ * they have duration=0 */
+ if (a->duration.seconds == 0 || b->duration.seconds == 0)
+ return NULL;
+
res = alloc_dive();
res->when = a->when;
diff --git a/dive.h b/dive.h
index cc27ab8..a10639a 100644
--- a/dive.h
+++ b/dive.h
@@ -233,9 +233,15 @@ struct event {
#define MAX_WEIGHTSYSTEMS (4)
#define W_IDX_PRIMARY 0
#define W_IDX_SECONDARY 1
+#define NO_TRIP -1
+#define NEXT_TRIP(entry, list) ((entry) ? g_list_next(entry) : (list))
+#define PREV_TRIP(entry, list) ((entry) ? g_list_previous(entry) : g_list_last(list))
+
+extern GList *dive_trip_list;
struct dive {
int number;
+ int tripid;
int selected;
time_t when;
char *location;
diff --git a/divelist.c b/divelist.c
index ef34b06..fd7687a 100644
--- a/divelist.c
+++ b/divelist.c
@@ -31,6 +31,7 @@ struct DiveList {
};
static struct DiveList dive_list;
+GList *dive_trip_list;
/*
* The dive list has the dive data in both string format (for showing)
@@ -56,17 +57,23 @@ enum {
/* magic numbers that indicate (as negative values) model entries that
* are summary entries for a divetrip */
-#define NEW_TRIP 1
#ifdef DEBUG_MODEL
static gboolean dump_model_entry(GtkTreeModel *model, GtkTreePath *path,
GtkTreeIter *iter, gpointer data)
{
char *location;
- int idx, nr, rating, depth;
+ int idx, nr, duration;
+ struct dive *dive;
+
+ gtk_tree_model_get(model, iter, DIVE_INDEX, &idx, DIVE_NR, &nr, DIVE_DURATION, &duration, DIVE_LOCATION, &location, -1);
+ printf("entry #%d : nr %d duration %d location %s ", idx, nr, duration, location);
+ dive = get_dive(idx);
+ if (dive)
+ printf("tripid %d\n", dive->tripid);
+ else
+ printf("without matching dive\n");
- gtk_tree_model_get(model, iter, DIVE_INDEX, &idx, DIVE_NR, &nr, DIVE_RATING, &rating, DIVE_DEPTH, &depth, DIVE_LOCATION, &location, -1);
- printf("entry #%d : nr %d rating %d depth %d location %s \n", idx, nr, rating, depth, location);
free(location);
return FALSE;
@@ -327,16 +334,14 @@ static void date_data_func(GtkTreeViewColumn *col,
when = val;
tm = gmtime(&when);
- switch(idx) {
- case -NEW_TRIP:
+ if (idx < 0) {
snprintf(buffer, sizeof(buffer),
"Trip %s, %s %d, %d (%d dive%s)",
weekday(tm->tm_wday),
monthname(tm->tm_mon),
tm->tm_mday, tm->tm_year + 1900,
nr, nr > 1 ? "s" : "");
- break;
- default:
+ } else {
snprintf(buffer, sizeof(buffer),
"%s, %s %d, %d %02d:%02d",
weekday(tm->tm_wday),
@@ -901,14 +906,64 @@ static int new_group(struct dive *dive, struct dive **last_dive, time_t *tm_date
return TRUE;
}
+static gboolean cleanup_tripids(GtkTreeModel *model, GtkTreePath *path, GtkTreeIter *iter, gpointer data)
+{
+ int idx, duration;
+ char *location;
+ static int current_tripid = -1;
+ static GList *trip = NULL;
+ struct dive *dive, *dive_trip;
+
+ gtk_tree_model_get(model, iter, DIVE_INDEX, &idx, DIVE_DURATION, &duration, DIVE_LOCATION, &location, -1);
+ printf("idx %3d ",idx);
+ if (duration == 0) {
+ printf("trip (current %d) ", current_tripid);
+ if (idx >= current_tripid) {
+ idx = --current_tripid;
+ printf("now %d ", current_tripid);
+ gtk_tree_store_set(GTK_TREE_STORE(dive_list.treemodel), iter, DIVE_INDEX, idx, -1);
+ }
+ trip = PREV_TRIP(trip, dive_trip_list);
+ if (! trip) {
+ printf("how can we run out of trips here?\n");
+ return FALSE;
+ }
+ dive_trip = trip->data;
+ dive_trip->tripid = idx;
+ if (location)
+ dive_trip->location = strdup(location);
+ printf("at %s\n", location ? location : "no location");
+ free(location);
+
+ return FALSE;
+ }
+ if (idx < 0) {
+ printf("corrupt table - negative index and positive duration!\n");
+ exit(1);
+ }
+ dive = get_dive(idx);
+ if (!dive) {
+ printf("corrupt table - no dive for index %d\n", idx);
+ exit(1);
+ }
+ printf("dive (tripid %d) location %s\n", dive->tripid, location);
+ free(location);
+ if (dive->tripid == -1)
+ return FALSE;
+ dive->tripid = current_tripid;
+
+ return FALSE;
+}
+
static void fill_dive_list(void)
{
- int i, group_size;
- GtkTreeIter iter, parent_iter;
+ int i;
+ GtkTreeIter iter, parent_iter, *parent_ptr = NULL;
GtkTreeStore *liststore, *treestore;
+ int last_tripid = 0;
struct dive *last_dive = NULL;
- struct dive *last_trip_dive = NULL;
- const char *last_location = NULL;
+ struct dive *dive_trip = NULL;
+ GList *trip = NULL;
time_t dive_date;
treestore = GTK_TREE_STORE(dive_list.treemodel);
@@ -916,36 +971,75 @@ static void fill_dive_list(void)
i = dive_table.nr;
while (--i >= 0) {
- struct dive *dive = dive_table.dives[i];
+ struct dive *dive = get_dive(i);
+
+ if (!dive)
+ continue;
- if (new_group(dive, &last_dive, &dive_date))
+ /* tripid defines how dives are grouped;
+ * == 0 means "not handled yet" - for dives with 0 created time based group
+ * == -1 means "set as no group" - simply leave at top level
+ * < -1 means "belongs to trip nr tripid
+ */
+ if ((dive->tripid == 0 && new_group(dive, &last_dive, &dive_date)) ||
+ dive->tripid != last_tripid)
{
/* make sure we display the first date of the trip in previous summary */
- if (last_trip_dive)
- gtk_tree_store_set(treestore, &parent_iter,
- DIVE_NR, group_size,
- DIVE_DATE, last_trip_dive->when,
- DIVE_LOCATION, last_location,
- -1);
-
- gtk_tree_store_append(treestore, &parent_iter, NULL);
- gtk_tree_store_set(treestore, &parent_iter,
- DIVE_INDEX, -NEW_TRIP,
- DIVE_NR, 1,
- DIVE_TEMPERATURE, 0,
- DIVE_SAC, 0,
+ if (dive_trip && parent_ptr) {
+ gtk_tree_store_set(treestore, parent_ptr,
+ DIVE_NR, dive_trip->number,
+ DIVE_DATE, dive_trip->when,
+ DIVE_LOCATION, dive_trip->location,
-1);
+ }
- group_size = 0;
- /* This might be NULL */
- last_location = dive->location;
+ if (dive->tripid != -1) {
+ gtk_tree_store_append(treestore, &parent_iter, NULL);
+ parent_ptr = &parent_iter;
+
+ if (dive->tripid == 0) {
+ /* allocate new trip - all fields default to 0 */
+ dive_trip = alloc_dive();
+ dive_trip->when = dive->when;
+ dive_trip->number = 0; /* we increment below */
+ dive_trip->location = dive->location;
+ dive_trip_list = g_list_prepend(dive_trip_list, dive_trip);
+ }
+ /* do we have a trip for this? */
+ trip = PREV_TRIP(trip, dive_trip_list);
+ if (!trip) {
+ printf("oops, no trip?\n");
+ }
+ dive_trip = trip->data;
+ if (dive->tripid != dive_trip->tripid)
+ /* GAAA - inconsistent data */
+ printf("input data inconsistent; dive asked for tripid %d but next trip is %d\n",
+ dive->tripid, dive_trip->tripid);
+ /* a duration of 0 identifies a group; if this was newly created
+ * based on the new_group algorithm then the tripid will still be 0;
+ * we clean that up after all dives are read */
+ gtk_tree_store_set(treestore, parent_ptr,
+ DIVE_INDEX, dive->tripid,
+ DIVE_NR, 1,
+ DIVE_DURATION, 0,
+ DIVE_TEMPERATURE, 0,
+ DIVE_SAC, 0,
+ -1);
+ } else {
+ /* dives with tripid = -1 are at top level */
+ parent_ptr = NULL;
+ dive_trip = NULL;
+ }
+ last_tripid = dive->tripid;
+ }
+ if (dive_trip) {
+ dive_trip->number++;
+ dive_trip->when = dive->when;
+ if (dive->location)
+ dive_trip->location = dive->location;
}
- group_size++;
- last_trip_dive = dive;
- if (dive->location)
- last_location = dive->location;
update_cylinder_related_info(dive);
- gtk_tree_store_append(treestore, &iter, &parent_iter);
+ gtk_tree_store_append(treestore, &iter, parent_ptr);
gtk_tree_store_set(treestore, &iter,
DIVE_INDEX, i,
DIVE_NR, dive->number,
@@ -974,12 +1068,14 @@ static void fill_dive_list(void)
}
/* make sure we display the first date of the trip in previous summary */
- if (last_trip_dive)
- gtk_tree_store_set(treestore, &parent_iter,
- DIVE_NR, group_size,
- DIVE_DATE, last_trip_dive->when,
- DIVE_LOCATION, last_location,
+ if (parent_ptr && dive_trip)
+ gtk_tree_store_set(treestore, parent_ptr,
+ DIVE_NR, dive_trip->number,
+ DIVE_DATE, dive_trip->when,
+ DIVE_LOCATION, dive_trip->location,
-1);
+ /* clean up all the tripids */
+ gtk_tree_model_foreach(GTK_TREE_MODEL(treestore), cleanup_tripids, NULL);
update_dive_list_units();
if (gtk_tree_model_get_iter_first(GTK_TREE_MODEL(dive_list.model), &iter)) {
diff --git a/parse-xml.c b/parse-xml.c
index 173314d..62d37d1 100644
--- a/parse-xml.c
+++ b/parse-xml.c
@@ -24,7 +24,15 @@ struct dive_table dive_table;
*/
void record_dive(struct dive *dive)
{
- int nr = dive_table.nr, allocated = dive_table.allocated;
+ int nr, allocated;
+
+ if (dive->duration.seconds == 0) {
+ /* this is a trip */
+ dive_trip_list = g_list_append(dive_trip_list, dive);
+ return;
+ }
+ nr = dive_table.nr;
+ allocated = dive_table.allocated;
struct dive **dives = dive_table.dives;
if (nr >= allocated) {
@@ -1062,6 +1070,8 @@ static void try_to_fill_dive(struct dive **divep, const char *name, char *buf)
if (MATCH(".number", get_index, &dive->number))
return;
+ if (MATCH(".tripid", get_index, &dive->tripid))
+ return;
if (MATCH(".date", divedate, &dive->when))
return;
if (MATCH(".time", divetime, &dive->when))
diff --git a/save-xml.c b/save-xml.c
index 37d6d06..82cce18 100644
--- a/save-xml.c
+++ b/save-xml.c
@@ -283,14 +283,18 @@ static void save_dive(FILE *f, struct dive *dive)
fputs("<dive", f);
if (dive->number)
fprintf(f, " number='%d'", dive->number);
+ fprintf(f, " tripid='%d'", dive->tripid);
if (dive->rating)
fprintf(f, " rating='%d'", dive->rating);
fprintf(f, " date='%04u-%02u-%02u'",
tm->tm_year+1900, tm->tm_mon+1, tm->tm_mday);
fprintf(f, " time='%02u:%02u:%02u'",
tm->tm_hour, tm->tm_min, tm->tm_sec);
- fprintf(f, " duration='%u:%02u min'>\n",
- FRACTION(dive->duration.seconds, 60));
+ if (dive->duration.seconds)
+ fprintf(f, " duration='%u:%02u min'>\n",
+ FRACTION(dive->duration.seconds, 60));
+ else
+ fprintf(f, " tripentry='TRUE'>\n");
save_overview(f, dive);
save_cylinder_info(f, dive);
save_weightsystem_info(f, dive);
@@ -305,6 +309,9 @@ static void save_dive(FILE *f, struct dive *dive)
void save_dives(const char *filename)
{
int i;
+ int last_tripid = NO_TRIP;
+ GList *trip = NULL;
+
FILE *f = fopen(filename, "w");
if (!f)
@@ -314,8 +321,18 @@ void save_dives(const char *filename)
update_dive(current_dive);
fprintf(f, "<dives>\n<program name='subsurface' version='%d'></program>\n", VERSION);
- for (i = 0; i < dive_table.nr; i++)
+ for (i = 0; i < dive_table.nr; i++) {
+ struct dive *dive = get_dive(i);
+ if (dive->tripid != last_tripid && dive->tripid != NO_TRIP) {
+ trip = NEXT_TRIP(trip, dive_trip_list);
+ if (trip->data)
+ save_dive(f, trip->data);
+ last_tripid = dive->tripid;
+ printf("trip %d\n",last_tripid);
+ }
save_dive(f, get_dive(i));
+ printf ("dive %d\n", get_dive(i)->number);
+ }
fprintf(f, "</dives>\n");
fclose(f);
}
More information about the subsurface
mailing list