From b9d76845521fdba09a41955231b705004c7bacb6 Mon Sep 17 00:00:00 2001 From: Jef Driesen Date: Mon, 27 Nov 2023 21:34:12 +0100 Subject: [PATCH] Refactor the code to read the ringbuffer pointers Move the code to read the ringbuffer pointers into a separate function that deals with all the quirks internally and simply returns two begin/end pairs. To obtain the logbook end pointer, the page size is now added to the value of the last pointer without taking into account the ringbuffer boundaries. Therefore the boundary checks in the caller need to be relaxed to accept the end pointer as a valid value. The profile begin/end pointers are not used anywhere and are only retrieved for diagnostics purposes. --- src/oceanic_atom2.c | 1 + src/oceanic_common.c | 97 +++++++++++++++++++++++++++++--------------- src/oceanic_common.h | 10 ++++- src/oceanic_veo250.c | 1 + src/oceanic_vtpro.c | 89 +++++++++++++++++++++++++++++----------- 5 files changed, 140 insertions(+), 58 deletions(-) diff --git a/src/oceanic_atom2.c b/src/oceanic_atom2.c index 2b90448..d520e0e 100644 --- a/src/oceanic_atom2.c +++ b/src/oceanic_atom2.c @@ -87,6 +87,7 @@ static const oceanic_common_device_vtable_t oceanic_atom2_device_vtable = { oceanic_atom2_device_close /* close */ }, oceanic_common_device_devinfo, + oceanic_common_device_pointers, oceanic_common_device_logbook, oceanic_common_device_profile, }; diff --git a/src/oceanic_common.c b/src/oceanic_common.c index b13c289..f9edc7c 100644 --- a/src/oceanic_common.c +++ b/src/oceanic_common.c @@ -32,8 +32,7 @@ #define VTABLE(abstract) ((const oceanic_common_device_vtable_t *) abstract->vtable) -#define RB_LOGBOOK_DISTANCE(a,b,l) ringbuffer_distance (a, b, DC_RINGBUFFER_FULL, l->rb_logbook_begin, l->rb_logbook_end) -#define RB_LOGBOOK_INCR(a,b,l) ringbuffer_increment (a, b, l->rb_logbook_begin, l->rb_logbook_end) +#define RB_LOGBOOK_DISTANCE(a,b,l,m) ringbuffer_distance (a, b, m, l->rb_logbook_begin, l->rb_logbook_end) #define RB_PROFILE_DISTANCE(a,b,l,m) ringbuffer_distance (a, b, m, l->rb_profile_begin, l->rb_profile_end) @@ -263,7 +262,51 @@ oceanic_common_device_devinfo (dc_device_t *abstract, dc_event_progress_t *progr dc_status_t -oceanic_common_device_logbook (dc_device_t *abstract, dc_event_progress_t *progress, dc_buffer_t *logbook) +oceanic_common_device_pointers (dc_device_t *abstract, dc_event_progress_t *progress, + unsigned int *rb_logbook_begin, unsigned int *rb_logbook_end, + unsigned int *rb_profile_begin, unsigned int *rb_profile_end) +{ + dc_status_t status = DC_STATUS_SUCCESS; + oceanic_common_device_t *device = (oceanic_common_device_t *) abstract; + + assert (device != NULL); + assert (device->layout != NULL); + assert (rb_logbook_begin != NULL && rb_logbook_end != NULL); + assert (rb_profile_begin != NULL && rb_profile_end != NULL); + + const oceanic_common_layout_t *layout = device->layout; + + // Read the pointer data. + unsigned char pointers[PAGESIZE] = {0}; + status = dc_device_read (abstract, layout->cf_pointers, pointers, sizeof (pointers)); + if (status != DC_STATUS_SUCCESS) { + ERROR (abstract->context, "Failed to read the memory page."); + return status; + } + + // Update and emit a progress event. + if (progress) { + progress->current += PAGESIZE; + progress->maximum += PAGESIZE; + device_event_emit (abstract, DC_EVENT_PROGRESS, progress); + } + + // Get the pointers. + unsigned int rb_logbook_first = array_uint16_le (pointers + 4); + unsigned int rb_logbook_last = array_uint16_le (pointers + 6); + unsigned int rb_profile_first = array_uint16_le (pointers + 8); + unsigned int rb_profile_last = array_uint16_le (pointers + 10); + + *rb_logbook_begin = rb_logbook_first; + *rb_logbook_end = rb_logbook_last + (layout->pt_mode_global == 0 ? layout->rb_logbook_entry_size : 0); + *rb_profile_begin = rb_profile_first; + *rb_profile_end = rb_profile_last; + + return status; +} + +dc_status_t +oceanic_common_device_logbook (dc_device_t *abstract, dc_event_progress_t *progress, dc_buffer_t *logbook, unsigned int begin, unsigned int end) { oceanic_common_device_t *device = (oceanic_common_device_t *) abstract; dc_status_t rc = DC_STATUS_SUCCESS; @@ -279,32 +322,16 @@ oceanic_common_device_logbook (dc_device_t *abstract, dc_event_progress_t *progr if (!dc_buffer_clear (logbook)) return DC_STATUS_NOMEMORY; - // Read the pointer data. - unsigned char pointers[PAGESIZE] = {0}; - rc = dc_device_read (abstract, layout->cf_pointers, pointers, sizeof (pointers)); - if (rc != DC_STATUS_SUCCESS) { - ERROR (abstract->context, "Failed to read the memory page."); - return rc; - } - - // Get the logbook pointers. - unsigned int rb_logbook_first = array_uint16_le (pointers + 4); - unsigned int rb_logbook_last = array_uint16_le (pointers + 6); - if (rb_logbook_last < layout->rb_logbook_begin || - rb_logbook_last >= layout->rb_logbook_end) + // Validate the logbook pointers. + unsigned int rb_logbook_begin = begin; + unsigned int rb_logbook_end = end; + if (rb_logbook_end < layout->rb_logbook_begin || + rb_logbook_end > layout->rb_logbook_end) { - ERROR (abstract->context, "Invalid logbook end pointer detected (0x%04x).", rb_logbook_last); + ERROR (abstract->context, "Invalid logbook end pointer detected (0x%04x).", rb_logbook_end); return DC_STATUS_DATAFORMAT; } - // Calculate the end pointer. - unsigned int rb_logbook_end = 0; - if (layout->pt_mode_global == 0) { - rb_logbook_end = RB_LOGBOOK_INCR (rb_logbook_last, layout->rb_logbook_entry_size, layout); - } else { - rb_logbook_end = rb_logbook_last; - } - // Calculate the number of bytes. // In a typical ringbuffer implementation with only two begin/end // pointers, there is no distinction possible between an empty and a @@ -312,20 +339,18 @@ oceanic_common_device_logbook (dc_device_t *abstract, dc_event_progress_t *progr // case, because an empty ringbuffer can be detected by inspecting // the logbook entries once they are downloaded. unsigned int rb_logbook_size = 0; - if (rb_logbook_first < layout->rb_logbook_begin || - rb_logbook_first >= layout->rb_logbook_end) + if (rb_logbook_begin < layout->rb_logbook_begin || + rb_logbook_begin > layout->rb_logbook_end) { // Fall back to downloading the entire logbook ringbuffer as // workaround for an invalid logbook begin pointer! - ERROR (abstract->context, "Invalid logbook begin pointer detected (0x%04x).", rb_logbook_first); + ERROR (abstract->context, "Invalid logbook begin pointer detected (0x%04x).", rb_logbook_begin); rb_logbook_size = layout->rb_logbook_end - layout->rb_logbook_begin; } else { - rb_logbook_size = RB_LOGBOOK_DISTANCE (rb_logbook_first, rb_logbook_end, layout); + rb_logbook_size = RB_LOGBOOK_DISTANCE (rb_logbook_begin, rb_logbook_end, layout, DC_RINGBUFFER_FULL); } // Update and emit a progress event. - progress->current += PAGESIZE; - progress->maximum += PAGESIZE; progress->maximum -= (layout->rb_logbook_end - layout->rb_logbook_begin) - rb_logbook_size; device_event_emit (abstract, DC_EVENT_PROGRESS, progress); @@ -642,6 +667,14 @@ oceanic_common_device_foreach (dc_device_t *abstract, dc_dive_callback_t callbac return rc; } + // Read the ringbuffer pointers. + unsigned int rb_logbook_begin = 0, rb_logbook_end = 0; + unsigned int rb_profile_begin = 0, rb_profile_end = 0; + rc = VTABLE(abstract)->pointers (abstract, &progress, &rb_logbook_begin, &rb_logbook_end, &rb_profile_begin, &rb_profile_end); + if (rc != DC_STATUS_SUCCESS) { + return rc; + } + // Memory buffer for the logbook data. dc_buffer_t *logbook = dc_buffer_new (0); if (logbook == NULL) { @@ -649,7 +682,7 @@ oceanic_common_device_foreach (dc_device_t *abstract, dc_dive_callback_t callbac } // Download the logbook ringbuffer. - rc = VTABLE(abstract)->logbook (abstract, &progress, logbook); + rc = VTABLE(abstract)->logbook (abstract, &progress, logbook, rb_logbook_begin, rb_logbook_end); if (rc != DC_STATUS_SUCCESS) { dc_buffer_free (logbook); return rc; diff --git a/src/oceanic_common.h b/src/oceanic_common.h index 307b6a2..b4291f7 100644 --- a/src/oceanic_common.h +++ b/src/oceanic_common.h @@ -169,7 +169,8 @@ typedef struct oceanic_common_device_t { typedef struct oceanic_common_device_vtable_t { dc_device_vtable_t base; dc_status_t (*devinfo) (dc_device_t *device, dc_event_progress_t *progress); - dc_status_t (*logbook) (dc_device_t *device, dc_event_progress_t *progress, dc_buffer_t *logbook); + dc_status_t (*pointers) (dc_device_t *device, dc_event_progress_t *progress, unsigned int *rb_logbook_begin, unsigned int *rb_logbook_end, unsigned int *rb_profile_begin, unsigned int *rb_profile_end); + dc_status_t (*logbook) (dc_device_t *device, dc_event_progress_t *progress, dc_buffer_t *logbook, unsigned int begin, unsigned int end); dc_status_t (*profile) (dc_device_t *device, dc_event_progress_t *progress, dc_buffer_t *logbook, dc_dive_callback_t callback, void *userdata); } oceanic_common_device_vtable_t; @@ -190,7 +191,12 @@ dc_status_t oceanic_common_device_devinfo (dc_device_t *device, dc_event_progress_t *progress); dc_status_t -oceanic_common_device_logbook (dc_device_t *device, dc_event_progress_t *progress, dc_buffer_t *logbook); +oceanic_common_device_pointers (dc_device_t *device, dc_event_progress_t *progress, + unsigned int *rb_logbook_begin, unsigned int *rb_logbook_end, + unsigned int *rb_profile_begin, unsigned int *rb_profile_end); + +dc_status_t +oceanic_common_device_logbook (dc_device_t *device, dc_event_progress_t *progress, dc_buffer_t *logbook, unsigned int begin, unsigned int end); dc_status_t oceanic_common_device_profile (dc_device_t *device, dc_event_progress_t *progress, dc_buffer_t *logbook, dc_dive_callback_t callback, void *userdata); diff --git a/src/oceanic_veo250.c b/src/oceanic_veo250.c index b4030ad..82f73c7 100644 --- a/src/oceanic_veo250.c +++ b/src/oceanic_veo250.c @@ -59,6 +59,7 @@ static const oceanic_common_device_vtable_t oceanic_veo250_device_vtable = { oceanic_veo250_device_close /* close */ }, oceanic_common_device_devinfo, + oceanic_common_device_pointers, oceanic_common_device_logbook, oceanic_common_device_profile, }; diff --git a/src/oceanic_vtpro.c b/src/oceanic_vtpro.c index 40afe80..cba7fcc 100644 --- a/src/oceanic_vtpro.c +++ b/src/oceanic_vtpro.c @@ -51,7 +51,8 @@ typedef struct oceanic_vtpro_device_t { oceanic_vtpro_protocol_t protocol; } oceanic_vtpro_device_t; -static dc_status_t oceanic_vtpro_device_logbook (dc_device_t *abstract, dc_event_progress_t *progress, dc_buffer_t *logbook); +static dc_status_t oceanic_vtpro_device_pointers (dc_device_t *abstract, dc_event_progress_t *progress, unsigned int *rb_logbook_begin, unsigned int *rb_logbook_end, unsigned int *rb_profile_begin, unsigned int *rb_profile_end); +static dc_status_t oceanic_vtpro_device_logbook (dc_device_t *abstract, dc_event_progress_t *progress, dc_buffer_t *logbook, unsigned int begin, unsigned int end); static dc_status_t oceanic_vtpro_device_read (dc_device_t *abstract, unsigned int address, unsigned char data[], unsigned int size); static dc_status_t oceanic_vtpro_device_close (dc_device_t *abstract); @@ -68,6 +69,7 @@ static const oceanic_common_device_vtable_t oceanic_vtpro_device_vtable = { oceanic_vtpro_device_close /* close */ }, oceanic_common_device_devinfo, + oceanic_vtpro_device_pointers, oceanic_vtpro_device_logbook, oceanic_common_device_profile, }; @@ -287,7 +289,49 @@ oceanic_vtpro_calibrate (oceanic_vtpro_device_t *device) } static dc_status_t -oceanic_aeris500ai_device_logbook (dc_device_t *abstract, dc_event_progress_t *progress, dc_buffer_t *logbook) +oceanic_aeris500ai_device_pointers (dc_device_t *abstract, dc_event_progress_t *progress, unsigned int *rb_logbook_begin, unsigned int *rb_logbook_end, unsigned int *rb_profile_begin, unsigned int *rb_profile_end) +{ + dc_status_t status = DC_STATUS_SUCCESS; + oceanic_vtpro_device_t *device = (oceanic_vtpro_device_t *) abstract; + + assert (device != NULL); + assert (device->base.layout != NULL); + assert (rb_logbook_begin != NULL && rb_logbook_end != NULL); + assert (rb_profile_begin != NULL && rb_profile_end != NULL); + + const oceanic_common_layout_t *layout = device->base.layout; + + // Read the pointer data. + unsigned char pointers[PAGESIZE] = {0}; + status = oceanic_vtpro_device_read (abstract, layout->cf_pointers, pointers, sizeof (pointers)); + if (status != DC_STATUS_SUCCESS) { + ERROR (abstract->context, "Failed to read the memory page."); + return status; + } + + // Update and emit a progress event. + if (progress) { + progress->current += PAGESIZE; + progress->maximum += PAGESIZE; + device_event_emit (abstract, DC_EVENT_PROGRESS, progress); + } + + // Get the pointers. + unsigned int rb_logbook_first = pointers[0x02]; + unsigned int rb_logbook_last = pointers[0x03]; + unsigned int rb_profile_first = array_uint16_le (pointers + 4) * PAGESIZE; + unsigned int rb_profile_last = array_uint16_le (pointers + 6) * PAGESIZE; + + *rb_logbook_begin = rb_logbook_first; + *rb_logbook_end = rb_logbook_last; + *rb_profile_begin = rb_profile_first; + *rb_profile_end = rb_profile_last; + + return status; +} + +static dc_status_t +oceanic_aeris500ai_device_logbook (dc_device_t *abstract, dc_event_progress_t *progress, dc_buffer_t *logbook, unsigned int begin, unsigned int end) { dc_status_t rc = DC_STATUS_SUCCESS; oceanic_vtpro_device_t *device = (oceanic_vtpro_device_t *) abstract; @@ -298,30 +342,15 @@ oceanic_aeris500ai_device_logbook (dc_device_t *abstract, dc_event_progress_t *p assert (device->base.layout->rb_logbook_begin == device->base.layout->rb_logbook_end); assert (progress != NULL); - const oceanic_common_layout_t *layout = device->base.layout; - // Erase the buffer. if (!dc_buffer_clear (logbook)) return DC_STATUS_NOMEMORY; - // Read the pointer data. - unsigned char pointers[PAGESIZE] = {0}; - rc = oceanic_vtpro_device_read (abstract, layout->cf_pointers, pointers, sizeof (pointers)); - if (rc != DC_STATUS_SUCCESS) { - ERROR (abstract->context, "Failed to read the memory page."); - return rc; - } - - // Get the logbook pointers. - unsigned int first = pointers[0x02]; - unsigned int last = pointers[0x03]; - // Get the number of dives. - unsigned int ndives = last - first + 1; + unsigned int ndives = end - begin + 1; // Update and emit a progress event. - progress->current += PAGESIZE; - progress->maximum += PAGESIZE + ndives * PAGESIZE / 2; + progress->maximum += ndives * PAGESIZE / 2; device_event_emit (abstract, DC_EVENT_PROGRESS, progress); // Allocate memory for the logbook entries. @@ -330,8 +359,8 @@ oceanic_aeris500ai_device_logbook (dc_device_t *abstract, dc_event_progress_t *p // Send the logbook index command. unsigned char command[] = {0x52, - first & 0xFF, - last & 0xFF, + begin & 0xFF, + end & 0xFF, 0x00}; rc = oceanic_vtpro_transfer (device, command, sizeof (command), NULL, 0); if (rc != DC_STATUS_SUCCESS) { @@ -379,14 +408,26 @@ oceanic_aeris500ai_device_logbook (dc_device_t *abstract, dc_event_progress_t *p } static dc_status_t -oceanic_vtpro_device_logbook (dc_device_t *abstract, dc_event_progress_t *progress, dc_buffer_t *logbook) +oceanic_vtpro_device_pointers (dc_device_t *abstract, dc_event_progress_t *progress, unsigned int *rb_logbook_begin, unsigned int *rb_logbook_end, unsigned int *rb_profile_begin, unsigned int *rb_profile_end) { oceanic_vtpro_device_t *device = (oceanic_vtpro_device_t *) abstract; if (device->base.model == AERIS500AI) { - return oceanic_aeris500ai_device_logbook (abstract, progress, logbook); + return oceanic_aeris500ai_device_pointers (abstract, progress, rb_logbook_begin, rb_logbook_end, rb_profile_begin, rb_profile_end); } else { - return oceanic_common_device_logbook (abstract, progress, logbook); + return oceanic_common_device_pointers (abstract, progress, rb_logbook_begin, rb_logbook_end, rb_profile_begin, rb_profile_end); + } +} + +static dc_status_t +oceanic_vtpro_device_logbook (dc_device_t *abstract, dc_event_progress_t *progress, dc_buffer_t *logbook, unsigned int begin, unsigned int end) +{ + oceanic_vtpro_device_t *device = (oceanic_vtpro_device_t *) abstract; + + if (device->base.model == AERIS500AI) { + return oceanic_aeris500ai_device_logbook (abstract, progress, logbook, begin, end); + } else { + return oceanic_common_device_logbook (abstract, progress, logbook, begin, end); } }