Merge branch 'fix_gerror_pointer' into 'master'

mm-iface: fix GError pointer

See merge request mobian1/devices/eg25-manager!2

meson.build

@@ -8,7 +8,7 @@
 project (
   'eg25manager',
   'c',
-  version : '0.0.1',
+  version : '0.0.6',
   license : 'GPLv3+',
   meson_version : '>= 0.50.0',
   default_options :
@@ -44,6 +44,7 @@ mgr_deps = [
     dependency('glib-2.0'),
     dependency('gio-unix-2.0'),
     dependency('libgpiod'),
+    dependency('libusb-1.0'),
     dependency('mm-glib'),
 ]
 

src/at.c

@@ -31,7 +31,7 @@ static int configure_serial(const char *tty)
     struct termios ttycfg;
     int fd;
 
-    fd = open(tty, O_RDWR | O_NOCTTY);
+    fd = open(tty, O_RDWR | O_NOCTTY | O_NONBLOCK);
     if (fd > 0) {
         tcgetattr(fd, &ttycfg);
         ttycfg.c_iflag &= ~(IGNBRK | BRKINT | ICRNL | INLCR | PARMRK | INPCK | ISTRIP | IXON);
@@ -52,19 +52,24 @@ static int configure_serial(const char *tty)
 static gboolean send_at_command(struct EG25Manager *manager)
 {
     char command[256];
-    struct AtCommand *at_cmd = g_list_nth_data(manager->at_cmds, 0);
+    struct AtCommand *at_cmd = manager->at_cmds ? g_list_nth_data(manager->at_cmds, 0) : NULL;
+    int ret, len = 0;
 
     if (at_cmd) {
-        if (at_cmd->subcmd == NULL && at_cmd->value == NULL)
-            sprintf(command, "AT+%s?\r\n", at_cmd->cmd);
+        if (at_cmd->subcmd == NULL && at_cmd->value == NULL && at_cmd->expected == NULL)
+            len = sprintf(command, "AT+%s\r\n", at_cmd->cmd);
+        else if (at_cmd->subcmd == NULL && at_cmd->value == NULL)
+            len = sprintf(command, "AT+%s?\r\n", at_cmd->cmd);
         else if (at_cmd->subcmd == NULL && at_cmd->value)
-            sprintf(command, "AT+%s=%s\r\n", at_cmd->cmd, at_cmd->value);
+            len = sprintf(command, "AT+%s=%s\r\n", at_cmd->cmd, at_cmd->value);
         else if (at_cmd->subcmd && at_cmd->value == NULL)
-            sprintf(command, "AT+%s=\"%s\"\r\n", at_cmd->cmd, at_cmd->subcmd);
+            len = sprintf(command, "AT+%s=\"%s\"\r\n", at_cmd->cmd, at_cmd->subcmd);
         else if (at_cmd->subcmd && at_cmd->value)
-            sprintf(command, "AT+%s=\"%s\",%s\r\n", at_cmd->cmd, at_cmd->subcmd, at_cmd->value);
+            len = sprintf(command, "AT+%s=\"%s\",%s\r\n", at_cmd->cmd, at_cmd->subcmd, at_cmd->value);
 
-        write(manager->at_fd, command, strlen(command));
+        ret = write(manager->at_fd, command, len);
+        if (ret < len)
+            g_warning("Couldn't write full AT command: wrote %d/%d bytes", ret, len);
 
         g_message("Sending command: %s", g_strstrip(command));
     } else if (manager->modem_state < EG25_STATE_CONFIGURED) {
@@ -86,7 +91,10 @@ static gboolean send_at_command(struct EG25Manager *manager)
 
 static void next_at_command(struct EG25Manager *manager)
 {
-    struct AtCommand *at_cmd = g_list_nth_data(manager->at_cmds, 0);
+    struct AtCommand *at_cmd = manager->at_cmds ? g_list_nth_data(manager->at_cmds, 0) : NULL;
+
+    if (!at_cmd)
+        return;
 
     if (at_cmd->cmd)
         g_free(at_cmd->cmd);
@@ -104,7 +112,10 @@ static void next_at_command(struct EG25Manager *manager)
 
 static void retry_at_command(struct EG25Manager *manager)
 {
-    struct AtCommand *at_cmd = g_list_nth_data(manager->at_cmds, 0);
+    struct AtCommand *at_cmd = manager->at_cmds ? g_list_nth_data(manager->at_cmds, 0) : NULL;
+
+    if (!at_cmd)
+        return;
 
     at_cmd->retries++;
     if (at_cmd->retries > 3) {
@@ -117,7 +128,10 @@ static void retry_at_command(struct EG25Manager *manager)
 
 static void process_at_result(struct EG25Manager *manager, char *response)
 {
-    struct AtCommand *at_cmd = g_list_nth_data(manager->at_cmds, 0);
+    struct AtCommand *at_cmd = manager->at_cmds ? g_list_nth_data(manager->at_cmds, 0) : NULL;
+
+    if (!at_cmd)
+        return;
 
     if (at_cmd->expected && !strstr(response, at_cmd->expected)) {
         if (at_cmd->value)
@@ -143,7 +157,6 @@ static int append_at_command(struct EG25Manager *manager,
     if (!at_cmd)
         return -1;
 
-    at_cmd->retries = 0;
     at_cmd->cmd = g_strdup(cmd);
     if (subcmd)
         at_cmd->subcmd = g_strdup(subcmd);
@@ -157,22 +170,33 @@ static int append_at_command(struct EG25Manager *manager,
     return 0;
 }
 
+#define READ_BUFFER_SIZE 256
+
 static gboolean modem_response(gint fd,
                                GIOCondition event,
                                gpointer data)
 {
     struct EG25Manager *manager = data;
-    char response[256];
-    int ret;
+    char response[READ_BUFFER_SIZE*4+1];
+    char tmp[READ_BUFFER_SIZE];
+    ssize_t ret, pos = 0;
 
     /*
-     * TODO: several reads can be necessary to get the full response, we could
-     * loop until we read 0 chars with a reasonable delay between attempts
+     * Several reads can be necessary to get the full response, so we loop
+     * until we read 0 chars with a reasonable delay between attempts
      * (remember the transfer rate is 115200 here)
      */
-    ret = read(fd, response, sizeof(response));
+    do {
+        ret = read(fd, tmp, sizeof(tmp));
         if (ret > 0) {
-        response[ret] = 0;
+            memcpy(&response[pos], tmp, ret);
+            pos += ret;
+            usleep(10000);
+        }
+    } while (ret > 0 && pos < (sizeof(response) - 1));
+
+    if (pos > 0) {
+        response[pos] = 0;
         g_strstrip(response);
         if (strlen(response) == 0)
             return TRUE;
@@ -181,10 +205,13 @@ static gboolean modem_response(gint fd,
 
         if (strcmp(response, "RDY") == 0)
             manager->modem_state = EG25_STATE_STARTED;
-        else if (strcmp(response, "ERROR") == 0)
+        else if (strstr(response, "ERROR"))
             retry_at_command(manager);
         else if (strstr(response, "OK"))
             process_at_result(manager, response);
+        else
+            // Not a recognized response, try running next command, just in case
+            next_at_command(manager);
     }
 
     return TRUE;
@@ -206,6 +233,7 @@ int at_init(struct EG25Manager *manager)
 void at_destroy(struct EG25Manager *manager)
 {
     g_source_remove(manager->at_source);
+    if (manager->at_fd > 0)
         close(manager->at_fd);
 }
 
@@ -213,9 +241,9 @@ void at_sequence_configure(struct EG25Manager *manager)
 {
     /*
      * Default parameters in megi's driver which differ with our own:
-     *   - urc/ri/* are always set the same way on both BH and CE
-     *   - urc/ri/* pulse duration is 1 ms and urc/delay is 0 (no need to delay
-     *     URCs if the pulse is that short)
+     *   - urc/ri/x are always set the same way on both BH and CE
+     *   - urc/ri/x pulse duration is 1 ms and urc/delay is 0 (no need to delay
+     *     URCs if the pulse is that short, so this is expected)
      *   - apready is disabled
      *
      * Parameters set in megi's kernel but not here:
@@ -248,7 +276,7 @@ void at_sequence_configure(struct EG25Manager *manager)
 
 void at_sequence_suspend(struct EG25Manager *manager)
 {
-    append_at_command(manager, "QGPS", NULL, NULL, "0");
+    append_at_command(manager, "QGPSEND", NULL, NULL, NULL);
     append_at_command(manager, "QCFG", "urc/cache", "1", NULL);
     send_at_command(manager);
 }
@@ -256,7 +284,7 @@ void at_sequence_suspend(struct EG25Manager *manager)
 void at_sequence_resume(struct EG25Manager *manager)
 {
     append_at_command(manager, "QCFG", "urc/cache", "0", NULL);
-    append_at_command(manager, "QGPS", NULL, NULL, "1");
+    append_at_command(manager, "QGPS", NULL, "1", NULL);
     send_at_command(manager);
 }
 

src/gpio.c

@@ -57,7 +57,7 @@ int gpio_sequence_poweron(struct EG25Manager *manager)
     sleep(1);
     gpiod_line_set_value(manager->gpio_out[GPIO_OUT_PWRKEY], 0);
 
-    g_message("Executed power-on sequence");
+    g_message("Executed power-on/off sequence");
 
     return 0;
 }
@@ -131,13 +131,13 @@ int gpio_init(struct EG25Manager *manager)
 
         manager->gpio_out[i] = gpiod_chip_get_line(manager->gpiochip[chipidx], offset);
         if (!manager->gpio_out[i]) {
-            g_critical("Unable to get output GPIO %d", i);
+            g_error("Unable to get output GPIO %d", i);
             return 1;
         }
 
         ret = gpiod_line_request_output(manager->gpio_out[i], "eg25manager", 0);
         if (ret < 0) {
-            g_critical("Unable to request output GPIO %d", i);
+            g_error("Unable to request output GPIO %d", i);
             return 1;
         }
     }
@@ -172,13 +172,16 @@ int gpio_init(struct EG25Manager *manager)
     return 0;
 }
 
-gboolean gpio_check_poweroff(struct EG25Manager *manager)
+gboolean gpio_check_poweroff(struct EG25Manager *manager, gboolean keep_down)
 {
     if (manager->gpio_in[GPIO_IN_STATUS] &&
         gpiod_line_get_value(manager->gpio_in[GPIO_IN_STATUS]) == 1) {
 
+        if (keep_down && manager->gpio_out[GPIO_OUT_RESET]) {
             // Asserting RESET line to prevent modem from rebooting
             gpiod_line_set_value(manager->gpio_out[GPIO_OUT_RESET], 1);
+        }
+
         return TRUE;
     }
 
@@ -189,9 +192,17 @@ void gpio_destroy(struct EG25Manager *manager)
 {
     int i;
 
-    for (i = 0; i < GPIO_OUT_COUNT; i++)
+    for (i = 0; i < GPIO_OUT_COUNT; i++) {
+        if (manager->gpio_out[i])
             gpiod_line_release(manager->gpio_out[i]);
+    }
 
+    for (i = 0; i < GPIO_IN_COUNT; i++) {
+        if (manager->gpio_in[i])
+            gpiod_line_release(manager->gpio_in[i]);
+    }
+
+    if (manager->gpiochip[0])
         gpiod_chip_close(manager->gpiochip[0]);
     if (manager->gpiochip[1])
         gpiod_chip_close(manager->gpiochip[1]);

src/gpio.h

@@ -16,4 +16,4 @@ int gpio_sequence_shutdown(struct EG25Manager *state);
 int gpio_sequence_suspend(struct EG25Manager *state);
 int gpio_sequence_resume(struct EG25Manager *state);
 
-gboolean gpio_check_poweroff(struct EG25Manager *manager);
+gboolean gpio_check_poweroff(struct EG25Manager *manager, gboolean keep_down);

src/manager.c

@@ -16,49 +16,74 @@
 #include <unistd.h>
 
 #include <glib-unix.h>
+#include <libusb.h>
 
-static gboolean quit_timeout_cb(struct EG25Manager *manager)
-{
-    g_message("Modem down, quitting...");
-    g_main_loop_quit(manager->loop);
-
-    return FALSE;
-}
-
-static gboolean gpio_poll_cb(struct EG25Manager *manager)
-{
-    if (gpio_check_poweroff(manager)) {
-        quit_timeout_cb(manager);
-        return FALSE;
-    }
-
-    return TRUE;
-}
+#define EG25_USB_VID 0x2c7c
+#define EG25_USB_PID 0x0125
 
 static gboolean quit_app(struct EG25Manager *manager)
 {
+    int i;
+
     g_message("Request to quit...");
 
+    at_destroy(manager);
+    mm_iface_destroy(manager);
+    suspend_destroy(manager);
+
     if (manager->modem_state >= EG25_STATE_STARTED) {
         g_message("Powering down the modem...");
         gpio_sequence_shutdown(manager);
         manager->modem_state = EG25_STATE_FINISHING;
-        g_timeout_add(500, G_SOURCE_FUNC(gpio_poll_cb), manager);
-        g_timeout_add_seconds(30, G_SOURCE_FUNC(quit_timeout_cb), manager);
+        for (i = 0; i < 30; i++) {
+            if (gpio_check_poweroff(manager, TRUE))
+                break;
+            sleep(1);
         }
+    }
+    g_message("Modem down, quitting...");
 
-    mm_iface_destroy(manager);
-    suspend_destroy(manager);
-    g_bus_unwatch_name(manager->mm_watch);
+    g_main_loop_quit(manager->loop);
 
     return FALSE;
 }
 
 static gboolean modem_start(struct EG25Manager *manager)
 {
+    ssize_t i, count;
+    gboolean should_boot = TRUE;
+    libusb_context *ctx = NULL;
+    libusb_device **devices = NULL;
+    struct libusb_device_descriptor desc;
+
+    if (manager->braveheart) {
+        // BH don't have the STATUS line connected, so check if USB device is present
+        libusb_init(&ctx);
+
+        count = libusb_get_device_list(ctx, &devices);
+        for (i = 0; i < count; i++) {
+            libusb_get_device_descriptor(devices[i], &desc);
+            if (desc.idVendor == EG25_USB_VID && desc.idProduct == EG25_USB_PID) {
+                g_message("Found corresponding USB device, modem already powered");
+                should_boot = FALSE;
+                break;
+            }
+        }
+
+        libusb_free_device_list(devices, 1);
+        libusb_exit(ctx);
+    } else if (!gpio_check_poweroff(manager, FALSE)) {
+        g_message("STATUS is low, modem already powered");
+        should_boot = FALSE;
+    }
+
+    if (should_boot) {
         g_message("Starting modem...");
         gpio_sequence_poweron(manager);
         manager->modem_state = EG25_STATE_POWERED;
+    } else {
+        manager->modem_state = EG25_STATE_STARTED;
+    }
 
     return FALSE;
 }
@@ -87,21 +112,25 @@ void modem_configure(struct EG25Manager *manager)
 
 void modem_reset(struct EG25Manager *manager)
 {
-    int fd;
+    int fd, ret, len = strlen(manager->modem_usb_id);
 
     fd = open("/sys/bus/usb/drivers/usb/unbind", O_WRONLY);
     if (fd < 0)
         goto error;
-    write(fd, manager->modem_usb_id, strlen(manager->modem_usb_id));
+    ret = write(fd, manager->modem_usb_id, len);
+    if (ret < len)
+        g_warning("Couldn't unbind modem: wrote %d/%d bytes", ret, len);
     close(fd);
 
     fd = open("/sys/bus/usb/drivers/usb/bind", O_WRONLY);
     if (fd < 0)
         goto error;
-    write(fd, manager->modem_usb_id, strlen(manager->modem_usb_id));
+    ret = write(fd, manager->modem_usb_id, len);
+    if (ret < len)
+        g_warning("Couldn't unbind modem: wrote %d/%d bytes", ret, len);
+
     close(fd);
 
-    manager->modem_state = EG25_STATE_CONFIGURED;
     return;
 
 error:
@@ -130,7 +159,7 @@ int main(int argc, char *argv[])
 {
     struct EG25Manager manager;
     char compatible[32];
-    int fd;
+    int fd, ret;
 
     memset(&manager, 0, sizeof(manager));
     manager.at_fd = -1;
@@ -143,8 +172,8 @@ int main(int argc, char *argv[])
         g_critical("Unable to read 'compatible' string from device tree");
         return 1;
     }
-    read(fd, compatible, sizeof(compatible));
-    if (!strstr(compatible, "pine64,pinephone-1.2"))
+    ret = read(fd, compatible, sizeof(compatible));
+    if (ret > 0 && !strstr(compatible, "pine64,pinephone-1.2"))
         manager.braveheart = TRUE;
     close(fd);
 
@@ -160,7 +189,6 @@ int main(int argc, char *argv[])
 
     g_main_loop_run(manager.loop);
 
-    at_destroy(&manager);
     gpio_destroy(&manager);
 
     return 0;

src/mm-iface.c

@@ -47,6 +47,9 @@ static void add_modem(struct EG25Manager *manager, GDBusObject *object)
         modem_configure(manager);
 
     path = mm_modem_get_device(manager->mm_modem);
+
+    if (manager->modem_usb_id)
+        g_free(manager->modem_usb_id);
     manager->modem_usb_id = g_strdup(strrchr(path, '/') + 1);
 
     gdbus_modem = MM_GDBUS_MODEM(manager->mm_modem);
@@ -81,13 +84,8 @@ static void interface_removed_cb(struct EG25Manager *manager,
 
     g_message("ModemManager interface `%s' removed on object `%s'", info->name, path);
 
-    if (g_strcmp0(info->name, MM_DBUS_INTERFACE_MODEM) == 0) {
+    if (g_strcmp0(info->name, MM_DBUS_INTERFACE_MODEM) == 0)
         manager->mm_modem = NULL;
-        if (manager->modem_usb_id) {
-            g_free(manager->modem_usb_id);
-            manager->modem_usb_id = NULL;
-        }
-    }
 }
 
 
@@ -130,10 +128,6 @@ static void object_removed_cb(struct EG25Manager *manager, GDBusObject *object)
     g_message("ModemManager object `%s' removed", path);
 
     manager->mm_modem = NULL;
-    if (manager->modem_usb_id) {
-        g_free(manager->modem_usb_id);
-        manager->modem_usb_id = NULL;
-    }
 }
 
 
@@ -141,7 +135,7 @@ static void mm_manager_new_cb(GDBusConnection    *connection,
                               GAsyncResult       *res,
                               struct EG25Manager *manager)
 {
-    GError *error = NULL;
+    g_autoptr (GError) error = NULL;
 
     manager->mm_manager = mm_manager_new_finish(res, &error);
     if (!manager->mm_manager)
@@ -189,5 +183,16 @@ void mm_iface_init(struct EG25Manager *manager)
 
 void mm_iface_destroy(struct EG25Manager *manager)
 {
+    if (manager->mm_manager) {
         g_clear_object(&manager->mm_manager);
+        manager->mm_manager = NULL;
+    }
+    if (manager->modem_usb_id) {
+        g_free(manager->modem_usb_id);
+        manager->modem_usb_id = NULL;
+    }
+    if (manager->mm_watch != 0) {
+        g_bus_unwatch_name(manager->mm_watch);
+        manager->mm_watch = 0;
+    }
 }

src/suspend.c

@@ -42,14 +42,13 @@ static void inhibit_done(GObject *source,
 {
     GDBusProxy *suspend_proxy = G_DBUS_PROXY(source);
     struct EG25Manager *manager = user_data;
-    GError *error = NULL;
+    g_autoptr (GError) error = NULL;
     GVariant *res;
     GUnixFDList *fd_list;
 
     res = g_dbus_proxy_call_with_unix_fd_list_finish(suspend_proxy, &fd_list, result, &error);
     if (!res) {
         g_warning("inhibit failed: %s", error->message);
-        g_error_free(error);
     } else {
         if (!fd_list || g_unix_fd_list_get_length(fd_list) != 1)
             g_warning("didn't get a single fd back");
@@ -66,7 +65,8 @@ static void take_inhibitor(struct EG25Manager *manager)
 {
     GVariant *variant_arg;
 
-    g_assert(manager->suspend_inhibit_fd == -1);
+    if(manager->suspend_inhibit_fd != -1)
+        drop_inhibitor(manager);
 
     variant_arg = g_variant_new ("(ssss)", "sleep", "eg25manager",
                                  "eg25manager needs to prepare modem for sleep", "delay");
@@ -114,24 +114,24 @@ static void name_owner_cb(GObject *object,
     g_assert(proxy == manager->suspend_proxy);
 
     owner = g_dbus_proxy_get_name_owner(proxy);
-    if (owner)
+    if (owner) {
         take_inhibitor(manager);
-    else
-        drop_inhibitor(manager);
         g_free(owner);
+    } else {
+        drop_inhibitor(manager);
+    }
 }
 
 static void on_proxy_acquired(GObject *object,
                               GAsyncResult *res,
                               struct EG25Manager *manager)
 {
-    GError *error = NULL;
+    g_autoptr (GError) error = NULL;
     char *owner;
 
     manager->suspend_proxy = g_dbus_proxy_new_for_bus_finish(res, &error);
     if (!manager->suspend_proxy) {
         g_warning("failed to acquire logind proxy: %s", error->message);
-        g_clear_error(&error);
         return;
     }
 
@@ -139,10 +139,11 @@ static void on_proxy_acquired(GObject *object,
     g_signal_connect(manager->suspend_proxy, "g-signal", G_CALLBACK(signal_cb), manager);
 
     owner = g_dbus_proxy_get_name_owner(manager->suspend_proxy);
-    if (owner)
+    if (owner) {
         take_inhibitor(manager);
         g_free(owner);
     }
+}
 
 void suspend_init(struct EG25Manager *manager)
 {
@@ -156,7 +157,10 @@ void suspend_init(struct EG25Manager *manager)
 void suspend_destroy(struct EG25Manager *manager)
 {
     drop_inhibitor(manager);
+    if (manager->suspend_proxy) {
         g_object_unref(manager->suspend_proxy);
+        manager->suspend_proxy = NULL;
+    }
 }
 
 void suspend_inhibit(struct EG25Manager *manager, gboolean inhibit)