Hey Tomas,
See comments below.
On 02/02/16 15:30, [email protected] wrote:
From: Tomas Jura <[email protected]>
Looks like that previous email address doesn't exist? Earlier messages
sent there bounced with error.
---
plugins/mbm/mm-broadband-modem-mbm.c | 24 +++++++++++++++++++++++-
1 file changed, 23 insertions(+), 1 deletion(-)
diff --git a/plugins/mbm/mm-broadband-modem-mbm.c
b/plugins/mbm/mm-broadband-modem-mbm.c
index ec4563c..2f0a014 100644
--- a/plugins/mbm/mm-broadband-modem-mbm.c
+++ b/plugins/mbm/mm-broadband-modem-mbm.c
@@ -1348,6 +1348,16 @@ enable_location_gathering_finish (MMIfaceModemLocation
*self,
return !g_simple_async_result_propagate_error (G_SIMPLE_ASYNC_RESULT
(res), error);
}
+static void close_unmanaged_port ( MMPortSerialGps *self,
"static void" should go in its own line.
Also, "self" in this context should be only used for the modem object;
I'd suggest you use "port" or something like that instead.
+ GAsyncResult *res,
+ LocationGatheringContext *ctx )
+{
Now that I see it, a call to mm_port_serial_command_finish () is missing
here. Every time a callback is provided to an async method (instead of
just NULL) we should call the corresponding _finish() method. I don't
think it is strictly required in this specific case, because the
internal GSimpleAsyncResult owns the response buffer, but for
consistency I think we should do it.
+ mm_port_serial_close(MM_PORT_SERIAL(self));
+ g_simple_async_result_set_op_res_gboolean (G_SIMPLE_ASYNC_RESULT(res),
TRUE);
+ location_gathering_context_complete_and_free (ctx);
+}
+
+
static void
gps_enabled_ready (MMBaseModem *self,
GAsyncResult *res,
@@ -1364,7 +1374,8 @@ gps_enabled_ready (MMBaseModem *self,
/* Only use the GPS port in NMEA/RAW setups */
if (ctx->source & (MM_MODEM_LOCATION_SOURCE_GPS_NMEA |
- MM_MODEM_LOCATION_SOURCE_GPS_RAW)) {
+ MM_MODEM_LOCATION_SOURCE_GPS_RAW |
+ MM_MODEM_LOCATION_SOURCE_GPS_UNMANAGED )) {
gps_port = mm_base_modem_peek_port_gps (self);
if (!gps_port ||
!mm_port_serial_open (MM_PORT_SERIAL (gps_port), &error)) {
@@ -1386,6 +1397,17 @@ gps_enabled_ready (MMBaseModem *self,
*/
buf = g_byte_array_new ();
g_byte_array_append (buf, (const guint8 *) command, strlen
(command));
+ if (ctx->source & MM_MODEM_LOCATION_SOURCE_GPS_UNMANAGED ) {
+ mm_port_serial_command (MM_PORT_SERIAL (gps_port),
+ buf,
+ 3,
+ FALSE,
+ NULL,
+ (GAsyncReadyCallback)
close_unmanaged_port,
+ ctx );
+ g_byte_array_unref (buf);
+ return;
+ }
mm_port_serial_command (MM_PORT_SERIAL (gps_port),
buf,
3,
As said in the original patch review, I think we should always provide a
GAsyncReadyCallback, not only when it's UNMANAGED, e.g.:
static void
e2gpsnpd_ready (MMPortSerialGps *port,
GAsyncResult *res,
LocationGatheringContext *ctx)
{
GError *error = NULL;
GByteArray *response;
response = mm_port_serial_command_finish (port, res, &error);
/* We ignore errors, just debug log them */
if (error) {
mm_dbg ("GPS enabling failed: %s", error->message);
g_error_free (error);
} else {
mm_dbg ("GPS enabling succeeded");
g_byte_array_unref (response);
}
if (ctx->source & MM_MODEM_LOCATION_SOURCE_GPS_UNMANAGED)
mm_port_serial_close (MM_PORT_SERIAL (self));
g_simple_async_result_set_op_res_gboolean
(G_SIMPLE_ASYNC_RESULT(res), TRUE);
location_gathering_context_complete_and_free (ctx);
}
...
mm_port_serial_command (MM_PORT_SERIAL (gps_port),
buf,
3,
FALSE,
NULL,
(GAsyncReadyCallback) e2gpsnpd_ready,
ctx);
