From 8d31e39e8951dae35edffffb7378d0661f5dc766 Mon Sep 17 00:00:00 2001 From: Arnaud Ferraris Date: Fri, 11 Dec 2020 11:42:51 +0100 Subject: [PATCH 1/5] manager: only start the modem if it isn't already on --- meson.build | 1 + src/gpio.c | 11 +++++++---- src/gpio.h | 2 +- src/manager.c | 43 +++++++++++++++++++++++++++++++++++++++---- 4 files changed, 48 insertions(+), 9 deletions(-) diff --git a/meson.build b/meson.build index 734eaa8..9834573 100644 --- a/meson.build +++ b/meson.build @@ -44,6 +44,7 @@ mgr_deps = [ dependency('glib-2.0'), dependency('gio-unix-2.0'), dependency('libgpiod'), + dependency('libusb-1.0'), dependency('mm-glib'), ] diff --git a/src/gpio.c b/src/gpio.c index fac7358..d22b30d 100644 --- a/src/gpio.c +++ b/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; } @@ -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) { - // Asserting RESET line to prevent modem from rebooting - gpiod_line_set_value(manager->gpio_out[GPIO_OUT_RESET], 1); + if (keep_down) { + // Asserting RESET line to prevent modem from rebooting + gpiod_line_set_value(manager->gpio_out[GPIO_OUT_RESET], 1); + } + return TRUE; } diff --git a/src/gpio.h b/src/gpio.h index a31c1ce..1f1efa6 100644 --- a/src/gpio.h +++ b/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); diff --git a/src/manager.c b/src/manager.c index fa75741..2b466e3 100644 --- a/src/manager.c +++ b/src/manager.c @@ -16,6 +16,10 @@ #include #include +#include + +#define EG25_USB_VID 0x2c7c +#define EG25_USB_PID 0x0125 static gboolean quit_app(struct EG25Manager *manager) { @@ -32,7 +36,7 @@ static gboolean quit_app(struct EG25Manager *manager) gpio_sequence_shutdown(manager); manager->modem_state = EG25_STATE_FINISHING; for (i = 0; i < 30; i++) { - if (gpio_check_poweroff(manager)) + if (gpio_check_poweroff(manager, TRUE)) break; sleep(1); } @@ -46,9 +50,40 @@ static gboolean quit_app(struct EG25Manager *manager) static gboolean modem_start(struct EG25Manager *manager) { - g_message("Starting modem..."); - gpio_sequence_poweron(manager); - manager->modem_state = EG25_STATE_POWERED; + 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; } From ff9b26b8315709b99f97b33e3119394190b2e7ed Mon Sep 17 00:00:00 2001 From: Arnaud Ferraris Date: Fri, 11 Dec 2020 11:56:14 +0100 Subject: [PATCH 2/5] manager: split modem_suspend() into _pre() and _post() functions This way we can make sure the AT commands are executed only once ModemManager has released the modem, preventing any race condition. --- src/manager.c | 8 +++++++- src/manager.h | 3 ++- src/mm-iface.c | 3 +++ src/suspend.c | 4 +--- 4 files changed, 13 insertions(+), 5 deletions(-) diff --git a/src/manager.c b/src/manager.c index 2b466e3..78bf98b 100644 --- a/src/manager.c +++ b/src/manager.c @@ -135,14 +135,20 @@ error: manager->modem_state = EG25_STATE_RESETTING; } -void modem_suspend(struct EG25Manager *manager) +void modem_suspend_pre(struct EG25Manager *manager) { + manager->modem_state = EG25_STATE_SUSPENDING; gpio_sequence_suspend(manager); +} + +void modem_suspend_post(struct EG25Manager *manager) +{ at_sequence_suspend(manager); } void modem_resume_pre(struct EG25Manager *manager) { + manager->modem_state = EG25_STATE_RESUMING; gpio_sequence_resume(manager); } diff --git a/src/manager.h b/src/manager.h index 714a812..be92a09 100644 --- a/src/manager.h +++ b/src/manager.h @@ -51,7 +51,8 @@ struct EG25Manager { void modem_configure(struct EG25Manager *data); void modem_reset(struct EG25Manager *data); -void modem_suspend(struct EG25Manager *data); +void modem_suspend_pre(struct EG25Manager *data); +void modem_suspend_post(struct EG25Manager *data); void modem_resume_pre(struct EG25Manager *data); void modem_resume_post(struct EG25Manager *data); void modem_update_state(struct EG25Manager *data, MMModemState state); diff --git a/src/mm-iface.c b/src/mm-iface.c index f1021de..9bf9209 100644 --- a/src/mm-iface.c +++ b/src/mm-iface.c @@ -132,6 +132,9 @@ static void object_removed_cb(struct EG25Manager *manager, GDBusObject *object) path = g_dbus_object_get_object_path(object); g_message("ModemManager object `%s' removed", path); + if (manager->modem_state == EG25_STATE_SUSPENDING) + modem_suspend_post(manager); + manager->mm_modem = NULL; if (manager->modem_usb_id) { g_free(manager->modem_usb_id); diff --git a/src/suspend.c b/src/suspend.c index 8296696..42a18f7 100644 --- a/src/suspend.c +++ b/src/suspend.c @@ -92,13 +92,11 @@ static void signal_cb(GDBusProxy *proxy, if (is_about_to_suspend) { g_message("system is about to suspend"); - manager->modem_state = EG25_STATE_SUSPENDING; - modem_suspend(manager); + modem_suspend_pre(manager); } else { g_message("system is resuming"); take_inhibitor(manager); modem_resume_pre(manager); - manager->modem_state = EG25_STATE_RESUMING; manager->suspend_source = g_timeout_add_seconds(8, G_SOURCE_FUNC(check_modem_resume), manager); } } From aabe4df41c9bbf3f80b91a170104d4e05bb59067 Mon Sep 17 00:00:00 2001 From: Arnaud Ferraris Date: Fri, 11 Dec 2020 12:15:32 +0100 Subject: [PATCH 3/5] at: fix suspend/resume sequences These are set commands, no need to verify the current value. --- src/at.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/at.c b/src/at.c index 87c7030..7e53c52 100644 --- a/src/at.c +++ b/src/at.c @@ -249,7 +249,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, "QGPS", NULL, "0", NULL); append_at_command(manager, "QCFG", "urc/cache", "1", NULL); send_at_command(manager); } @@ -257,7 +257,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); } From 5bc8443c381d4165e487ca99749ccd52d291ed26 Mon Sep 17 00:00:00 2001 From: Arnaud Ferraris Date: Fri, 11 Dec 2020 12:33:23 +0100 Subject: [PATCH 4/5] at: be less strict when checking for error The response can include the command and an error number, so we want to only check it contains ERROR, even if it's replying more than just the 'ERROR' string. --- src/at.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/at.c b/src/at.c index 7e53c52..069fed0 100644 --- a/src/at.c +++ b/src/at.c @@ -181,7 +181,7 @@ 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); From f386d851fa70d7cc0497fc0ff419722d02152d7c Mon Sep 17 00:00:00 2001 From: Arnaud Ferraris Date: Fri, 11 Dec 2020 12:47:14 +0100 Subject: [PATCH 5/5] at: make sure we read the full response before processing it --- src/at.c | 28 +++++++++++++++++++--------- 1 file changed, 19 insertions(+), 9 deletions(-) diff --git a/src/at.c b/src/at.c index 069fed0..c0cd92b 100644 --- a/src/at.c +++ b/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); @@ -143,7 +143,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 +156,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)); - if (ret > 0) { - response[ret] = 0; + do { + ret = read(fd, tmp, sizeof(tmp)); + if (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;