upd ublox

non_catalog_apps/ublox/README.md

@@ -1,7 +1,105 @@
 # ublox
+
+![Flipper Zero connected to a u-blox GPS, running the u-blox app](screenshots/flipper_ublox.jpg)
+
 Flipper Zero app to read from a u-blox GPS over I2C. This app can
 display data, log a path to a KML file, and sync the Flipper's time to
 GPS time. Get the app and see more info on the Flipper app hub!
 
-This app used to reside in my "flipped" GitHub repository, but I made
-it much better and moved it here.
+For feature requests or bug reports, open an issue on this repository.
+
+# Usage
+This app is compatible with GPSes that follow the UBX protocol version
+15, so it is definitely compatible with 8-series GPSes, and probably 9
+and 10-series too. I've only tested the app with a u-blox SAM-M8Q
+module from SparkFun. 
+
+## Wiring
+Connect your GPS to the 3V3, GND, and I2C pins on the Flipper. If you
+have a Qwiic GPS and a cable, these are the colors:
+
+| Qwiic color | Flipper pin |
+|-------------|-------------|
+| Red         | 3V3         |
+| Black       | GND         |
+| Blue        | C1 (SDA)    |
+| Yellow      | C0 (SCL)    |
+
+# Features
+## Handheld data display
+This app offers two forms of data display: handheld and car. The
+handheld displays more info:
+
+![u-blox app handheld data display](screenshots/data_display_handheld.png)
+
+- `F/S`: Fix type and satellite number. Fix types:
+  - `N`: No fix
+  - `R`: Dead-reckoning only
+  - `2`: 2D fix
+  - `3`: 3D fix
+  - `G+D`: Dead-reckoning and GNSS combined
+  - `TO`: Time-only
+- `Od`: Odometer since last reset. The odometer is inside the GPS.
+- `Lat`: Latitude in degrees to 4 decimal digits
+- `Lon`: Longitude in degrees to 4 decimal digits
+- `Alt`: Altitude above sea level
+- `Head`: Heading of motion. This is calculated in the GPS by
+  tracking motion, so if you're not moving, it won't be remotely
+  accurate.
+- `Time`: Current **Flipper** time, which might be inaccurate!
+- `Batt`: Current Flipper battery charge.
+
+All fields that can be localized will be converted to imperial or
+metric units, depending on what you've set in Flipper's Settings.
+
+The log button (accessed by the right-hand button on the d-pad)
+indicates the logging state.
+
+## Car data display
+![u-blox app car data display](screenshots/data_display_car.png)
+
+The car data display only displays 3 fields, in big numbers. You could
+in theory use it while you're driving (but place it on the dashboard
+or something, don't let the Flipper become a hazard).
+
+- `Spd (km/s)` or `Spd (mph)`: Current ground speed in whatever units.
+- `Heading`: Heading of motion.
+- `Odo (mi)` or `Odo (km)`: Odometer distance since last reset.
+
+## Data display config
+Press the left button on the data display to open the configuration
+panel.
+
+- `Refresh Rate`: How often the data display should be updated
+- `Display Mode`: Handheld or car
+- `Backlight`: Keep the display backlight always on or let it run on
+  the default timeout
+- `Notify`: If on, blink the LED on data display updates
+- `Platform Model`: Model used by the GPS's "Navigation
+  Engine". "Portable" is fine for most applications, but the other
+  modes are there if you want them.
+- `Odo Mode`: Another input to the Navigation Engine, used to tune the
+  odometer calculation.
+- `Reset Odometer`: Reset the on-GPS odometer.
+
+## Logging
+Press the right button on the data display to start logging. You'll be
+prompted to input a filename, and then logging will start. Files are
+saved to `apps_data/ublox` on the SD card. The app logs to KML format,
+and you can view them with a tool like [this
+one](https://www.doogal.co.uk/KmlViewer). Press the right button again
+to stop logging.
+
+You cannot configure the GPS during logging, and leaving the data
+display will also stop logging. If the GPS disconnects or becomes
+unreachable, logging will stop and all data will be saved. The log
+file is flushed every 16 writes in case a disk problem arises.
+
+## Time synchronization
+![u-blox app time synchronization display](screenshots/sync_time.png)
+
+This will synchronize the Flipper's time to the GPS's time. This
+doesn't touch the hour, so that your timezone is preserved, but syncs
+the minute and second. The Flipper's clock will be up to one second
+off, because there's no pulse-per-second signal.
+

non_catalog_apps/ublox/application.fam

@@ -3,7 +3,6 @@ App(
     name="u-blox GPS",
     apptype=FlipperAppType.EXTERNAL,
     entry_point="ublox_app",
-    cdefines=["APP_UBLOX"],
     requires=[
         "gui",
 	"i2c",
@@ -12,7 +11,7 @@ App(
     ],
     stack_size=2 * 1024,
     order=20,
-    fap_version=(0, 2), # major, minor
+    fap_version=(0, 3), # major, minor
     fap_description="App to display and log data from u-blox GPS modules over I2C",
     fap_author="liamur",
     fap_icon="ublox_app_icon.png",

non_catalog_apps/ublox/changelog.md

@@ -1,3 +1,7 @@
+0.3:
+- Bugfixes: logging and error modes are much less broken
+- New features: battery percentage on the data display and automatic
+  sync of the log file every 16 samples.
 0.1:
 - Initial release with data display, two view modes, time syncing, GPS
   configuration, and KML logging.

non_catalog_apps/ublox/helpers/kml.c

@@ -5,7 +5,9 @@
 bool kml_open_file(Storage* storage, KMLFile* kml, const char* path) {
     kml->file = storage_file_alloc(storage);
     if(!storage_file_open(kml->file, path, FSAM_WRITE, FSOM_CREATE_ALWAYS)) {
-        FURI_LOG_E(TAG, "failed to open KML file %s", path);
+	// must call close() even if the operation fails
+        FURI_LOG_E(TAG, "failed to open KML file!");
+	storage_file_close(kml->file);
         storage_file_free(kml->file);
         return false;
     }
@@ -35,11 +37,15 @@ bool kml_open_file(Storage* storage, KMLFile* kml, const char* path) {
                             "        <coordinates>\n";
 
     if(!storage_file_write(kml->file, kml_intro, strlen(kml_intro))) {
+	FURI_LOG_E(TAG, "failed to write KML starting header!");
         storage_file_close(kml->file);
         storage_file_free(kml->file);
         return false;
     }
 
+    // keeps track of writes for periodic flushes
+    kml->write_counter = 0;
+    FURI_LOG_I(TAG, "file opened successfully");
     return true;
 }
 
@@ -47,9 +53,21 @@ bool kml_add_path_point(KMLFile* kml, double lat, double lon, uint32_t alt) {
     // KML is longitude then latitude for some reason
     FuriString* point = furi_string_alloc_printf("          %f,%f,%lu\n", lon, lat, alt);
     if(!storage_file_write(kml->file, furi_string_get_cstr(point), furi_string_size(point))) {
+	FURI_LOG_E(TAG, "failed to write line to KML file!");
         return false;
     }
 
+    furi_string_free(point);
+
+    kml->write_counter += 1;
+    if (kml->write_counter == 16) {
+	if (!storage_file_sync(kml->file)) {
+	    FURI_LOG_E(TAG, "failed to periodic flush file!");
+	}
+	// reset
+	kml->write_counter = 0;
+    }
+    
     return true;
 }
 
@@ -60,7 +78,9 @@ bool kml_close_file(KMLFile* kml) {
                             "  </Document>\n"
                             "</kml>";
 
+    
     if(!storage_file_write(kml->file, kml_outro, strlen(kml_outro))) {
+	FURI_LOG_E(TAG, "failed to close KML file!");
         storage_file_close(kml->file);
         storage_file_free(kml->file);
         return false;

non_catalog_apps/ublox/helpers/kml.h

@@ -7,6 +7,7 @@
 
 typedef struct KMLFile {
     File* file;
+    int write_counter;
 } KMLFile;
 
 /**

non_catalog_apps/ublox/helpers/ublox_types.h

@@ -1,6 +1,6 @@
 #pragma once
 
-#define UBLOX_VERSION_APP "0.1"
+#define UBLOX_VERSION_APP "0.3"
 #define UBLOX_DEVELOPED "liamhays"
 #define UBLOX_GITHUB "https://github.com/liamhays/ublox"
 
@@ -48,8 +48,10 @@ typedef enum {
 
 typedef struct UbloxDataDisplayState {
     UbloxDataDisplayViewMode view_mode;
+    UbloxDataDisplayBacklightMode backlight_mode;
     UbloxDataDisplayRefreshRate refresh_rate;
     UbloxDataDisplayNotifyMode notify_mode;
+
 } UbloxDataDisplayState;
 
 typedef struct UbloxDeviceState {

non_catalog_apps/ublox/scenes/ublox_scene_about.c

@@ -33,17 +33,17 @@ void ublox_scene_about_on_enter(void* context) {
     furi_string_cat_printf(s, "\e#%s\n", "Description");
     furi_string_cat_printf(
         s,
-        "This app is a multi-purpose tool for u-blox GPS modules connected over I2C."
-        " It is compatible with 8 and 9 series GPS units, and probably other models,"
-        " sold by Sparkfun and other vendors.\n");
+        "App for u-blox GPS modules, compatible with 8 and 9 series GPS units.\n"
+	"Connect the GPS using I2C as shown on the Wiring page.\n");
 
     furi_string_cat_printf(s, "\e#%s\n", "Usage");
     furi_string_cat_printf(
         s,
         "Data Display shows GPS data. You can enable logging to a KML file to be"
-        " viewed in a map program.\n"
-        "Sync Time to GPS will sync the Flipper's RTC to the GPS. Note that this"
-        " may be up to one second off, because there is no PPS signal connected.");
+        " viewed in a map program."
+	" Logs are synced every 16 updates, and automatically save and stop when an error occurs.\n"
+        "Sync Time to GPS will sync the minute and second in the Flipper's RTC to the GPS."
+	" Note that this may be up to one second off, because there is no PPS signal connected.");
 
     widget_add_text_scroll_element(ublox->widget, 0, 16, 128, 50, furi_string_get_cstr(s));
 

non_catalog_apps/ublox/scenes/ublox_scene_data_display.c

@@ -42,16 +42,24 @@ void ublox_scene_data_display_on_enter(void* context) {
         ublox->worker, UbloxWorkerStateRead, ublox_scene_data_display_worker_callback, ublox);
 }
 
+// TODO: lock buttons feature
 bool ublox_scene_data_display_on_event(void* context, SceneManagerEvent event) {
     Ublox* ublox = context;
     bool consumed = false;
 
     if(event.type == SceneManagerEventTypeCustom) {
         if(event.event == GuiButtonTypeLeft) {
-            ublox_worker_stop(ublox->worker);
-            scene_manager_next_scene(ublox->scene_manager, UbloxSceneDataDisplayConfig);
-            consumed = true;
-
+	    // You can only config the worker when it's not logging,
+	    // because that's actually pretty complicated to implement
+	    // (and confusing too because the worker has to restart if
+	    // it's going to keep logging)
+
+	    if (ublox->log_state == UbloxLogStateNone) {
+		ublox_worker_stop(ublox->worker);
+		scene_manager_next_scene(ublox->scene_manager, UbloxSceneDataDisplayConfig);
+		consumed = true;
+	    }
+	    
         } else if(event.event == GuiButtonTypeRight) {
             if(data_display_get_state(ublox->data_display) != DataDisplayGPSNotFound) {
                 FURI_LOG_I(TAG, "right button");
@@ -67,6 +75,7 @@ bool ublox_scene_data_display_on_event(void* context, SceneManagerEvent event) {
             }
 
         } else if(event.event == UbloxWorkerEventDataReady) {
+
             if((ublox->data_display_state).notify_mode == UbloxDataDisplayNotifyOn) {
                 notification_message(ublox->notifications, &sequence_new_reading);
             }
@@ -85,6 +94,9 @@ bool ublox_scene_data_display_on_event(void* context, SceneManagerEvent event) {
         } else if(event.event == UbloxWorkerEventFailed) {
             FURI_LOG_I(TAG, "UbloxWorkerEventFailed");
             data_display_set_state(ublox->data_display, DataDisplayGPSNotFound);
+	    if(ublox->log_state == UbloxLogStateLogging) {
+		ublox->log_state = UbloxLogStateStopLogging;
+	    }
         }
     }
     return consumed;
@@ -93,13 +105,6 @@ bool ublox_scene_data_display_on_event(void* context, SceneManagerEvent event) {
 void ublox_scene_data_display_on_exit(void* context) {
     Ublox* ublox = context;
 
-    /*if(ublox->log_state == UbloxLogStateLogging) {
-	FURI_LOG_I(TAG, "stop logging on exit");
-	ublox->log_state = UbloxLogStateStopLogging;
-	//while (ublox->log_state != UbloxLogStateNone);
-	//furi_delay_ms(500);
-	}*/
-
     ublox_worker_stop(ublox->worker);
 
     data_display_reset(ublox->data_display);

non_catalog_apps/ublox/scenes/ublox_scene_data_display_config.c

@@ -27,6 +27,18 @@ const UbloxDataDisplayViewMode display_view_mode_value[DISPLAY_VIEW_MODE_COUNT]
     UbloxDataDisplayViewModeCar,
 };
 
+#define BACKLIGHT_MODE_COUNT 2
+const char* const backlight_mode_text[BACKLIGHT_MODE_COUNT] = {
+    "Default",
+    "On",
+
+};
+
+const UbloxDataDisplayBacklightMode backlight_mode_value[BACKLIGHT_MODE_COUNT] = {
+    UbloxDataDisplayBacklightDefault,
+    UbloxDataDisplayBacklightOn,
+};
+
 #define REFRESH_RATE_COUNT 8
 // double const means that the data is constant and that the pointer
 // is constant.
@@ -149,6 +161,38 @@ static void ublox_scene_data_display_config_set_display_view_mode(VariableItem*
     (ublox->data_display_state).view_mode = display_view_mode_value[index];
 }
 
+static uint8_t ublox_scene_data_display_config_next_backlight_mode(const UbloxDataDisplayBacklightMode value,
+								   void* context) {
+    furi_assert(context);
+
+    uint8_t index = 0;
+    for(int i = 0; i < BACKLIGHT_MODE_COUNT; i++) {
+	if(value == backlight_mode_value[i]) {
+            index = i;
+            break;
+        } else {
+            index = 0;
+        }
+    }
+    return index;
+}
+
+static void ublox_scene_data_display_config_set_backlight_mode(VariableItem* item) {
+    Ublox* ublox = variable_item_get_context(item);
+    uint8_t index = variable_item_get_current_value_index(item);
+
+    variable_item_set_current_value_text(item, backlight_mode_text[index]);
+    (ublox->data_display_state).backlight_mode = backlight_mode_value[index];
+
+    if((ublox->data_display_state).backlight_mode == UbloxDataDisplayBacklightOn) {
+	// backlight on
+	notification_message_block(ublox->notifications, &sequence_display_backlight_enforce_on);
+    } else if((ublox->data_display_state).backlight_mode == UbloxDataDisplayBacklightDefault) {
+	// backlight default
+	notification_message_block(ublox->notifications, &sequence_display_backlight_enforce_auto);
+    }
+}
+
 static uint8_t ublox_scene_data_display_config_next_notify_mode(
     const UbloxDataDisplayNotifyMode value,
     void* context) {
@@ -263,6 +307,17 @@ void ublox_scene_data_display_config_on_enter(void* context) {
     variable_item_set_current_value_index(item, value_index);
     variable_item_set_current_value_text(item, display_view_mode_text[value_index]);
 
+    item = variable_item_list_add(
+        ublox->variable_item_list,
+        "Backlight:",
+        BACKLIGHT_MODE_COUNT,
+        ublox_scene_data_display_config_set_backlight_mode,
+        ublox);
+    value_index = ublox_scene_data_display_config_next_backlight_mode(
+        (ublox->data_display_state).backlight_mode, ublox);
+    variable_item_set_current_value_index(item, value_index);
+    variable_item_set_current_value_text(item, backlight_mode_text[value_index]);
+
     item = variable_item_list_add(
         ublox->variable_item_list,
         "Notify:",

non_catalog_apps/ublox/ublox.c

@@ -64,14 +64,14 @@ Ublox* ublox_alloc() {
     ublox->storage = furi_record_open(RECORD_STORAGE);
 
     ublox->log_state = UbloxLogStateNone;
-    // default to "/data", which maps to "/ext/apps_data/ublox"
+    // files do actually belong here
     ublox->logfile_folder = furi_string_alloc_set(STORAGE_APP_DATA_PATH_PREFIX);
 
     // Establish default data display state
     (ublox->data_display_state).view_mode = UbloxDataDisplayViewModeHandheld;
     (ublox->data_display_state).refresh_rate = 2;
     (ublox->data_display_state).notify_mode = UbloxDataDisplayNotifyOn;
-
+    (ublox->data_display_state).backlight_mode = UbloxDataDisplayBacklightDefault;
     (ublox->device_state).odometer_mode = UbloxOdometerModeRunning;
     // "suitable for most applications" according to u-blox.
     (ublox->device_state).platform_model = UbloxPlatformModelPortable;
@@ -140,7 +140,9 @@ int32_t ublox_app(void* p) {
     // TODO: this is breaking the backlight timeout for everything
     // else: test by opening ublox, then leaving and opening DAP
     // Link. DAP Link should force the backlight on but doesn't.
-    notification_message_block(ublox->notifications, &sequence_display_backlight_enforce_auto);
+    if ((ublox->data_display_state).backlight_mode == UbloxDataDisplayBacklightOn) {
+	notification_message_block(ublox->notifications, &sequence_display_backlight_enforce_auto);
+    }
 
     ublox_free(ublox);
 

non_catalog_apps/ublox/ublox_device.c

@@ -1,6 +1,3 @@
-// This is a personal academic project. Dear PVS-Studio, please check it.
-
-// PVS-Studio Static Code Analyzer for C, C++, C#, and Java: https://pvs-studio.com
 #include "ublox_device.h"
 
 #define TAG "ublox_device"
@@ -134,3 +131,70 @@ void ublox_frame_free(UbloxFrame* frame) {
     FURI_LOG_I(TAG, "frame free: frame == NULL");
     }*/
 }
+
+UbloxMessage* ublox_i2c_transfer(UbloxMessage* message_tx, uint8_t read_length) {
+    if(!furi_hal_i2c_is_device_ready(
+           &furi_hal_i2c_handle_external,
+           UBLOX_I2C_ADDRESS << 1,
+           furi_ms_to_ticks(I2C_TIMEOUT_MS))) {
+        FURI_LOG_E(TAG, "device not ready");
+        return NULL;
+    }
+
+    // Either our I2C implementation is broken or the GPS's is, so we
+    // end up reading a lot more data than we need to. That means that
+    // the I2C comm code for this app is a little bit of a hack, but
+    // it works fine and is fast enough, so I don't really care. It
+    // certainly doesn't break the GPS.
+    if(!furi_hal_i2c_tx(
+           &furi_hal_i2c_handle_external,
+           UBLOX_I2C_ADDRESS << 1,
+           message_tx->message,
+           message_tx->length,
+           furi_ms_to_ticks(I2C_TIMEOUT_MS))) {
+        FURI_LOG_E(TAG, "error writing message to GPS");
+        return NULL;
+    }
+    uint8_t* response = malloc((size_t)read_length);
+    // The GPS sends 0xff until it has a complete message to respond
+    // with. We have to wait until it stops sending that. (Why this
+    // works is a little bit...uh, well, I don't know. Shouldn't reading
+    // more bytes make it so that the data is completely read out and no
+    // longer available?)
+
+    //FURI_LOG_I(TAG, "start ticks at %lu", furi_get_tick()); // returns ms
+    while(true) {
+        if(!furi_hal_i2c_rx(
+               &furi_hal_i2c_handle_external,
+               UBLOX_I2C_ADDRESS << 1,
+               response,
+               1,
+               furi_ms_to_ticks(I2C_TIMEOUT_MS))) {
+            FURI_LOG_E(TAG, "error reading first byte of response");
+            free(response);
+            return NULL;
+        }
+
+        // checking with 0xb5 prevents strange bursts of junk data from becoming an issue.
+        if(response[0] != 0xff && response[0] == 0xb5) {
+            //FURI_LOG_I(TAG, "read rest of message at %lu", furi_get_tick());
+            if(!furi_hal_i2c_rx(
+                   &furi_hal_i2c_handle_external,
+                   UBLOX_I2C_ADDRESS << 1,
+                   &(response[1]),
+                   read_length - 1, // first byte already read
+                   furi_ms_to_ticks(I2C_TIMEOUT_MS))) {
+                FURI_LOG_E(TAG, "error reading rest of response");
+                free(response);
+                return NULL;
+            }
+            break;
+        }
+        furi_delay_ms(1);
+    }
+
+    UbloxMessage* message_rx = malloc(sizeof(UbloxMessage));
+    message_rx->message = response;
+    message_rx->length = read_length;
+    return message_rx; // message_rx->message needs to be freed later
+}

non_catalog_apps/ublox/ublox_device.h

@@ -1,6 +1,3 @@
-// This is a personal academic project. Dear PVS-Studio, please check it.
-
-// PVS-Studio Static Code Analyzer for C, C++, C#, and Java: https://pvs-studio.com
 #pragma once
 
 #include <stdint.h>
@@ -8,6 +5,7 @@
 #include <stdbool.h>
 
 #include <furi.h>
+#include <furi_hal.h>
 
 #define UBLOX_I2C_ADDRESS 0x42
 #define I2C_TIMEOUT_MS 20
@@ -153,3 +151,5 @@ UbloxFrame* ublox_bytes_to_frame(UbloxMessage* message);
 
 void ublox_message_free(UbloxMessage* message);
 void ublox_frame_free(UbloxFrame* frame);
+
+UbloxMessage* ublox_i2c_transfer(UbloxMessage* message_tx, uint8_t read_length);

non_catalog_apps/ublox/ublox_i.h

@@ -53,6 +53,7 @@ struct Ublox {
     UbloxLogState log_state;
     FuriString* logfile_folder;
     char text_store[100];
+    
     Ublox_NAV_PVT_Message nav_pvt;
     Ublox_NAV_ODO_Message nav_odo;
     Ublox_NAV_TIMEUTC_Message nav_timeutc;

non_catalog_apps/ublox/ublox_worker.c

@@ -47,30 +47,23 @@ void ublox_worker_stop(UbloxWorker* ublox_worker) {
     furi_assert(ublox_worker->thread);
     Ublox* ublox = ublox_worker->context;
     furi_assert(ublox);
-    //FURI_LOG_I(TAG, "worker_stop: %p", ublox);
-
-    /*FuriThreadState state = furi_thread_get_state(ublox_worker->thread);
-  if (state == FuriThreadStateStopped) {
-      FURI_LOG_I(TAG, "worker state stopped");
-  } else if (state == FuriThreadStateStarting) {
-      FURI_LOG_I(TAG, "worker state starting");
-  } else if (state == FuriThreadStateRunning) {
-      FURI_LOG_I(TAG, "worker state running");
-      }*/
-
-    // close the logfile if currently logging
-    //FURI_LOG_I(TAG, "log state: %d", ublox->log_state);
-    if(ublox->log_state == UbloxLogStateLogging) {
-        FURI_LOG_I(TAG, "closing log file on worker stop");
-        ublox->log_state = UbloxLogStateNone;
-        if(!kml_close_file(&(ublox->kmlfile))) {
-            FURI_LOG_E(TAG, "failed to close KML file!");
-        }
-    }
+    
     if(furi_thread_get_state(ublox_worker->thread) != FuriThreadStateStopped) {
         ublox_worker_change_state(ublox_worker, UbloxWorkerStateStop);
         furi_thread_join(ublox_worker->thread);
     }
+
+    // Now that the worker thread is dead, we can access these
+    // safely. We have to have this separate from the nav_messages()
+    // function because of state.
+    if (ublox->log_state == UbloxLogStateLogging) {
+	FURI_LOG_I(TAG, "closing file in worker_stop()");
+	if(!kml_close_file(&(ublox->kmlfile))) {
+	    FURI_LOG_E(TAG, "failed to close KML file!");
+	}
+	// and revert the state
+	ublox->log_state = UbloxLogStateNone;
+    }
 }
 
 void ublox_worker_change_state(UbloxWorker* ublox_worker, UbloxWorkerState state) {
@@ -95,10 +88,8 @@ void clear_ublox_data() {
         if(!furi_hal_i2c_trx(
                &furi_hal_i2c_handle_external,
                UBLOX_I2C_ADDRESS << 1,
-               tx,
-               1,
-               &response,
-               1,
+               tx, 1,
+               &response, 1,
                furi_ms_to_ticks(I2C_TIMEOUT_MS))) {
             // if the GPS is disconnected during this loop, this will
             // loop forever, we must make that not happen. 30 loops is
@@ -113,13 +104,12 @@ void clear_ublox_data() {
 
 int32_t ublox_worker_task(void* context) {
     UbloxWorker* ublox_worker = context;
-
+    
     furi_hal_i2c_acquire(&furi_hal_i2c_handle_external);
 
     if(ublox_worker->state == UbloxWorkerStateRead) {
         ublox_worker_read_nav_messages(context);
     } else if(ublox_worker->state == UbloxWorkerStateSyncTime) {
-        FURI_LOG_I(TAG, "sync time");
         ublox_worker_sync_to_gps_time(ublox_worker);
     } else if(ublox_worker->state == UbloxWorkerStateResetOdometer) {
         ublox_worker_reset_odo(ublox_worker);
@@ -147,14 +137,17 @@ void ublox_worker_read_nav_messages(void* context) {
     // We only start logging at the same time we restart the worker.
     if(ublox->log_state == UbloxLogStateStartLogging) {
         FURI_LOG_I(TAG, "start logging");
+	
         // assemble full logfile pathname
-        FuriString* fullname = furi_string_alloc();
+	FuriString* fullname = furi_string_alloc();
         path_concat(furi_string_get_cstr(ublox->logfile_folder), ublox->text_store, fullname);
         FURI_LOG_I(TAG, "fullname is %s", furi_string_get_cstr(fullname));
 
         if(!kml_open_file(ublox->storage, &(ublox->kmlfile), furi_string_get_cstr(fullname))) {
             FURI_LOG_E(TAG, "failed to open KML file %s!", furi_string_get_cstr(fullname));
             ublox->log_state = UbloxLogStateNone;
+	    ublox_worker->callback(UbloxWorkerEventLogStateChanged, ublox_worker->context);
+	    return;
         }
         ublox->log_state = UbloxLogStateLogging;
         furi_string_free(fullname);
@@ -183,7 +176,6 @@ void ublox_worker_read_nav_messages(void* context) {
                 return;
             }
         }
-        //furi_delay_ms(500);
     }
 
     // clear data so we don't an error on startup
@@ -195,7 +187,7 @@ void ublox_worker_read_nav_messages(void* context) {
         uint32_t ticks = furi_get_tick();
         // we interrupt with checking the state to help reduce
         // lag. it's not perfect, but it does overall improve things.
-        bool pvt = ublox_worker_read_pvt(ublox_worker);
+        bool got_pvt = ublox_worker_read_pvt(ublox_worker);
 
         if(ublox_worker->state != UbloxWorkerStateRead) break;
         // clearing makes the second read much faster
@@ -203,11 +195,13 @@ void ublox_worker_read_nav_messages(void* context) {
 
         if(ublox_worker->state != UbloxWorkerStateRead) break;
 
-        bool odo = ublox_worker_read_odo(ublox_worker);
+        bool got_odo = ublox_worker_read_odo(ublox_worker);
 
-        if(pvt && odo) {
+        if(got_pvt && got_odo) {
+	    // if we got good data, do stuff
             ublox_worker->callback(UbloxWorkerEventDataReady, ublox_worker->context);
-
+	    FURI_LOG_I(TAG, "sent callback");
+	    // if logging, add point
             if(ublox->log_state == UbloxLogStateLogging) {
                 if(!kml_add_path_point(
                        &(ublox->kmlfile),
@@ -219,24 +213,30 @@ void ublox_worker_read_nav_messages(void* context) {
                 }
             }
         } else {
+	    // bad data
             ublox_worker->callback(UbloxWorkerEventFailed, ublox_worker->context);
         }
 
+	FURI_LOG_I(TAG, "going into loop");
         while(furi_get_tick() - ticks <
               furi_ms_to_ticks(((ublox->data_display_state).refresh_rate * 1000))) {
-            // putting this here (should) make the logging response faster
-            if(ublox->log_state == UbloxLogStateStopLogging) {
-                FURI_LOG_I(TAG, "stop logging");
-                if(!kml_close_file(&(ublox->kmlfile))) {
-                    FURI_LOG_E(TAG, "failed to close KML file!");
-                }
-                ublox->log_state = UbloxLogStateNone;
-                ublox_worker->callback(UbloxWorkerEventLogStateChanged, ublox_worker->context);
-            }
-            if(ublox_worker->state != UbloxWorkerStateRead) {
-                return;
-            }
+	    // putting these *inside* the loop makes it respond faster
+	    if(ublox_worker->state != UbloxWorkerStateRead) {
+		return;
+	    }
+
+	    // if logging stop is requested, do it
+	    if(ublox->log_state == UbloxLogStateStopLogging) {
+		FURI_LOG_I(TAG, "stop logging in tick loop");
+		if(!kml_close_file(&(ublox->kmlfile))) {
+		    FURI_LOG_E(TAG, "failed to close KML file!");
+		}
+		ublox->log_state = UbloxLogStateNone;
+		ublox_worker->callback(UbloxWorkerEventLogStateChanged, ublox_worker->context);
+	    }
         }
+	FURI_LOG_I(TAG, "finished loop");
+
     }
 }
 
@@ -252,7 +252,7 @@ void ublox_worker_sync_to_gps_time(void* context) {
     UbloxMessage* message_tx = ublox_frame_to_bytes(&frame_tx);
 
     UbloxMessage* message_rx =
-        ublox_worker_i2c_transfer(message_tx, UBX_NAV_TIMEUTC_MESSAGE_LENGTH);
+        ublox_i2c_transfer(message_tx, UBX_NAV_TIMEUTC_MESSAGE_LENGTH);
     if(message_rx == NULL) {
         FURI_LOG_E(TAG, "get_gps_time transfer failed");
         ublox_worker_change_state(ublox_worker, UbloxWorkerStateStop);
@@ -302,72 +302,6 @@ FuriString* print_uint8_array(uint8_t* array, int length) {
     return s;
 }
 
-UbloxMessage* ublox_worker_i2c_transfer(UbloxMessage* message_tx, uint8_t read_length) {
-    if(!furi_hal_i2c_is_device_ready(
-           &furi_hal_i2c_handle_external,
-           UBLOX_I2C_ADDRESS << 1,
-           furi_ms_to_ticks(I2C_TIMEOUT_MS))) {
-        FURI_LOG_E(TAG, "device not ready");
-        return NULL;
-    }
-
-    // Either our I2C implementation is broken or the GPS's is, so we
-    // end up reading a lot more data than we need to. That means that
-    // the I2C comm code for this app is a little bit of a hack, but
-    // it works fine and is fast enough, so I don't really care. It
-    // certainly doesn't break the GPS.
-    if(!furi_hal_i2c_tx(
-           &furi_hal_i2c_handle_external,
-           UBLOX_I2C_ADDRESS << 1,
-           message_tx->message,
-           message_tx->length,
-           furi_ms_to_ticks(I2C_TIMEOUT_MS))) {
-        FURI_LOG_E(TAG, "error writing message to GPS");
-        return NULL;
-    }
-    uint8_t* response = malloc((size_t)read_length);
-    // The GPS sends 0xff until it has a complete message to respond
-    // with. We have to wait until it stops sending that. (Why this
-    // works is a little bit...uh, well, I don't know. Shouldn't reading
-    // more bytes make it so that the data is completely read out and no
-    // longer available?)
-
-    //FURI_LOG_I(TAG, "start ticks at %lu", furi_get_tick()); // returns ms
-    while(true) {
-        if(!furi_hal_i2c_rx(
-               &furi_hal_i2c_handle_external,
-               UBLOX_I2C_ADDRESS << 1,
-               response,
-               1,
-               furi_ms_to_ticks(I2C_TIMEOUT_MS))) {
-            FURI_LOG_E(TAG, "error reading first byte of response");
-            free(response);
-            return NULL;
-        }
-
-        // checking with 0xb5 prevents strange bursts of junk data from becoming an issue.
-        if(response[0] != 0xff && response[0] == 0xb5) {
-            //FURI_LOG_I(TAG, "read rest of message at %lu", furi_get_tick());
-            if(!furi_hal_i2c_rx(
-                   &furi_hal_i2c_handle_external,
-                   UBLOX_I2C_ADDRESS << 1,
-                   &(response[1]),
-                   read_length - 1, // first byte already read
-                   furi_ms_to_ticks(I2C_TIMEOUT_MS))) {
-                FURI_LOG_E(TAG, "error reading rest of response");
-                free(response);
-                return NULL;
-            }
-            break;
-        }
-        furi_delay_ms(1);
-    }
-
-    UbloxMessage* message_rx = malloc(sizeof(UbloxMessage));
-    message_rx->message = response;
-    message_rx->length = read_length;
-    return message_rx; // message_rx->message needs to be freed later
-}
 
 bool ublox_worker_read_pvt(UbloxWorker* ublox_worker) {
     //FURI_LOG_I(TAG, "mem free before PVT read: %u", memmgr_get_free_heap());
@@ -382,7 +316,7 @@ bool ublox_worker_read_pvt(UbloxWorker* ublox_worker) {
     UbloxMessage* message_tx = ublox_frame_to_bytes(frame_tx);
     ublox_frame_free(frame_tx);
 
-    UbloxMessage* message_rx = ublox_worker_i2c_transfer(message_tx, UBX_NAV_PVT_MESSAGE_LENGTH);
+    UbloxMessage* message_rx = ublox_i2c_transfer(message_tx, UBX_NAV_PVT_MESSAGE_LENGTH);
     ublox_message_free(message_tx);
     if(message_rx == NULL) {
         FURI_LOG_E(TAG, "read_pvt transfer failed");
@@ -454,7 +388,6 @@ bool ublox_worker_read_pvt(UbloxWorker* ublox_worker) {
             .magDec = (frame_rx->payload[88]) | (frame_rx->payload[89] << 8),
             .magAcc = (frame_rx->payload[90]) | (frame_rx->payload[91] << 8),
         };
-
         // Using a local variable for nav_pvt is fine, because nav_pvt in
         // the Ublox struct is also not a pointer, so this assignment
         // effectively compiles to a memcpy.
@@ -475,7 +408,7 @@ bool ublox_worker_read_odo(UbloxWorker* ublox_worker) {
     UbloxMessage* message_tx = ublox_frame_to_bytes(frame_tx);
     ublox_frame_free(frame_tx);
 
-    UbloxMessage* message_rx = ublox_worker_i2c_transfer(message_tx, UBX_NAV_ODO_MESSAGE_LENGTH);
+    UbloxMessage* message_rx = ublox_i2c_transfer(message_tx, UBX_NAV_ODO_MESSAGE_LENGTH);
     ublox_message_free(message_tx);
     if(message_rx == NULL) {
         FURI_LOG_E(TAG, "read_odo transfer failed");
@@ -526,7 +459,7 @@ bool ublox_worker_init_gps(UbloxWorker* ublox_worker) {
     UbloxMessage* pms_message_tx = ublox_frame_to_bytes(&pms_frame_tx);
 
     UbloxMessage* pms_message_rx =
-        ublox_worker_i2c_transfer(pms_message_tx, UBX_CFG_PMS_MESSAGE_LENGTH);
+        ublox_i2c_transfer(pms_message_tx, UBX_CFG_PMS_MESSAGE_LENGTH);
     ublox_message_free(pms_message_tx);
     if(pms_message_rx == NULL) {
         FURI_LOG_E(TAG, "CFG-PMS read transfer failed");
@@ -543,7 +476,7 @@ bool ublox_worker_init_gps(UbloxWorker* ublox_worker) {
 
     pms_message_tx = ublox_frame_to_bytes(&pms_frame_tx);
 
-    UbloxMessage* ack = ublox_worker_i2c_transfer(pms_message_tx, UBX_ACK_ACK_MESSAGE_LENGTH);
+    UbloxMessage* ack = ublox_i2c_transfer(pms_message_tx, UBX_ACK_ACK_MESSAGE_LENGTH);
     if(ack == NULL) {
         FURI_LOG_E(TAG, "ACK after CFG-PMS set transfer failed");
         return false;
@@ -564,7 +497,7 @@ bool ublox_worker_init_gps(UbloxWorker* ublox_worker) {
     UbloxMessage* odo_message_tx = ublox_frame_to_bytes(&odo_frame_tx);
 
     UbloxMessage* odo_message_rx =
-        ublox_worker_i2c_transfer(odo_message_tx, UBX_CFG_ODO_MESSAGE_LENGTH);
+        ublox_i2c_transfer(odo_message_tx, UBX_CFG_ODO_MESSAGE_LENGTH);
     ublox_message_free(odo_message_tx);
     if(odo_message_rx == NULL) {
         FURI_LOG_E(TAG, "CFG-ODO transfer failed");
@@ -583,7 +516,7 @@ bool ublox_worker_init_gps(UbloxWorker* ublox_worker) {
 
     odo_message_tx = ublox_frame_to_bytes(&odo_frame_tx);
 
-    ack = ublox_worker_i2c_transfer(odo_message_tx, UBX_ACK_ACK_MESSAGE_LENGTH);
+    ack = ublox_i2c_transfer(odo_message_tx, UBX_ACK_ACK_MESSAGE_LENGTH);
     if(ack == NULL) {
         FURI_LOG_E(TAG, "ACK after CFG-ODO set transfer failed");
         return false;
@@ -604,7 +537,7 @@ bool ublox_worker_init_gps(UbloxWorker* ublox_worker) {
     UbloxMessage* nav5_message_tx = ublox_frame_to_bytes(&nav5_frame_tx);
 
     UbloxMessage* nav5_message_rx =
-        ublox_worker_i2c_transfer(nav5_message_tx, UBX_CFG_NAV5_MESSAGE_LENGTH);
+        ublox_i2c_transfer(nav5_message_tx, UBX_CFG_NAV5_MESSAGE_LENGTH);
     ublox_message_free(nav5_message_tx);
 
     if(nav5_message_rx == NULL) {
@@ -624,7 +557,7 @@ bool ublox_worker_init_gps(UbloxWorker* ublox_worker) {
 
     nav5_message_tx = ublox_frame_to_bytes(&nav5_frame_tx);
 
-    ack = ublox_worker_i2c_transfer(nav5_message_tx, UBX_ACK_ACK_MESSAGE_LENGTH);
+    ack = ublox_i2c_transfer(nav5_message_tx, UBX_ACK_ACK_MESSAGE_LENGTH);
     if(ack == NULL) {
         FURI_LOG_E(TAG, "ACK after CFG-NAV5 set transfer failed");
         return false;
@@ -647,7 +580,7 @@ void ublox_worker_reset_odo(UbloxWorker* ublox_worker) {
     odo_frame_tx.payload = NULL;
     UbloxMessage* odo_message_tx = ublox_frame_to_bytes(&odo_frame_tx);
 
-    UbloxMessage* ack = ublox_worker_i2c_transfer(odo_message_tx, UBX_ACK_ACK_MESSAGE_LENGTH);
+    UbloxMessage* ack = ublox_i2c_transfer(odo_message_tx, UBX_ACK_ACK_MESSAGE_LENGTH);
 
     ublox_message_free(odo_message_tx);
 

non_catalog_apps/ublox/ublox_worker.h

@@ -41,8 +41,6 @@ void ublox_worker_start(
 
 void ublox_worker_stop(UbloxWorker* ublox_worker);
 
-UbloxMessage* ublox_worker_i2c_transfer(UbloxMessage* message_tx, uint8_t read_length);
-
 bool ublox_worker_init_gps();
 
 void ublox_worker_read_nav_messages(void* context);

non_catalog_apps/ublox/views/data_display_view.c

@@ -19,10 +19,11 @@ typedef struct {
 
 static void draw_buttons(Canvas* canvas, void* model) {
     DataDisplayViewModel* m = model;
-    elements_button_left(canvas, "Config");
+
     if(m->log_state == UbloxLogStateLogging) {
         elements_button_right(canvas, "Stop Log");
     } else {
+	elements_button_left(canvas, "Config");
         elements_button_right(canvas, "Start Log");
     }
 }
@@ -80,7 +81,9 @@ static void data_display_draw_callback(Canvas* canvas, void* model) {
         }
         canvas_draw_str(canvas, 77, 9, furi_string_get_cstr(s));
 
-        canvas_set_font(canvas, FontPrimary);
+	// Former logging indicator
+	// We now use just the button as the indicator
+        /*canvas_set_font(canvas, FontPrimary);
         canvas_draw_str(canvas, 112, 9, "L:");
 
         canvas_set_font(canvas, FontSecondary);
@@ -88,7 +91,7 @@ static void data_display_draw_callback(Canvas* canvas, void* model) {
             canvas_draw_str(canvas, 122, 9, "Y"); // yes
         } else {
             canvas_draw_str(canvas, 122, 9, "N"); // no
-        }
+	    }*/
 
         /*** Draw latitude ***/
         canvas_set_font(canvas, FontPrimary);
@@ -127,33 +130,29 @@ static void data_display_draw_callback(Canvas* canvas, void* model) {
         furi_string_printf(s, "%.0f", (double)(message.headMot / 1e5));
         canvas_draw_str(canvas, 105, 35, furi_string_get_cstr(s));
 
-        /*** Draw time ***/
-        canvas_set_font(canvas, FontPrimary);
-        canvas_draw_str(canvas, 0, 48, "UTC:");
-
-        canvas_set_font(canvas, FontSecondary);
-        FuriHalRtcDateTime datetime = {
-            .hour = message.hour,
-            .minute = message.min,
-            .second = message.sec,
-        };
-
-        FuriString* s2 = furi_string_alloc();
-        // built-in date functions make strings that are too long
-        if(locale_get_date_format() == LocaleDateFormatDMY) {
-            furi_string_printf(s, "%u/%u/'%u ", message.day, message.month, (message.year % 100));
-        } else if(locale_get_date_format() == LocaleDateFormatMDY) {
-            furi_string_printf(s, "%u/%u/'%u ", message.month, message.day, (message.year % 100));
-        } else if(locale_get_date_format() == LocaleDateFormatYMD) {
-            furi_string_printf(s, "'%u/%u/%u ", (message.year % 100), message.month, message.day);
-        }
-        locale_format_time(s2, &datetime, locale_get_time_format(), false);
-        furi_string_cat(s, s2);
-        furi_string_free(s2);
-
-        canvas_draw_str(canvas, 27, 48, furi_string_get_cstr(s));
-        furi_string_free(s);
-
+        /*** Draw time and battery ***/
+	// Note that these are not retrieved from an external state
+	// variable, because they don't have to be.
+	canvas_set_font(canvas, FontPrimary);
+	canvas_draw_str(canvas, 0, 48, "Time:");
+	
+	FuriHalRtcDateTime datetime;
+	furi_hal_rtc_get_datetime(&datetime);
+	locale_format_time(s, &datetime, locale_get_time_format(), false);
+
+	canvas_set_font(canvas, FontSecondary);
+	canvas_draw_str(canvas, 30, 48, furi_string_get_cstr(s));
+
+	// draw Flipper battery charge
+	canvas_set_font(canvas, FontPrimary);
+	canvas_draw_str(canvas, 75, 48, "Batt:");
+
+	canvas_set_font(canvas, FontSecondary);
+	furi_string_printf(s, "%u%%", furi_hal_power_get_pct());
+	canvas_draw_str(canvas, 101, 48, furi_string_get_cstr(s));
+	
+	furi_string_free(s);
+	
     } else if(m->state == DataDisplayCarMode) {
         Ublox_NAV_PVT_Message message = m->nav_pvt;
         Ublox_NAV_ODO_Message nav_odo = m->nav_odo;