Comment on attachment 114562 mbm: add GPS location gathering support (v2) Review of attachment 114562: -----------------------------------------------------------------
Some minor additional things to fix. Also, could you maybe also attach a debug log of MM to see how it behaves with the GPS enabled/disabled? Just for reference, because I don't have a GPS capable MBM modem around. Thanks! ::: plugins/mbm/mm-broadband-modem-mbm.c @@ +1252,5 @@ > +typedef struct { > + MMBroadbandModemMbm *self; > + GSimpleAsyncResult *result; > + MMModemLocationSource source; > + int idx; Isn't this idx variable not needed? @@ +1388,5 @@ > + MM_CORE_ERROR, > + MM_CORE_ERROR_FAILED, > + "Couldn't open raw GPS > serial port"); > + } else { > + GByteArray *buf = g_byte_array_sized_new (13); A whiteline needed after the variable declaration. @@ +1389,5 @@ > + MM_CORE_ERROR_FAILED, > + "Couldn't open raw GPS > serial port"); > + } else { > + GByteArray *buf = g_byte_array_sized_new (13); > + g_byte_array_append(buf, (const guint8 *) "AT*E2GPSNPD\r\n", 12); I'm counting 13 chars in the string to be written, not 12. How about this? { GByteArray *buf; const gchar *command = "AT*E2GPSNPD\r\n"; buf = g_byte_array_new (); g_byte_array_append (buf, command, strlen (command)); mm_port_serial_command (...); g_byte_array_unref (buf); } Also, the fact that we need an AT command to be fed to the GPS data port is an unusual case (which is why you need to use the MMPortSerial API and cannot use the MMPortSerialAt one. I'd explain that in a comment message just around here, so that it's clear what is being done. -- You received this bug notification because you are a member of Ubuntu Bugs, which is subscribed to Ubuntu. https://bugs.launchpad.net/bugs/783220 Title: [needs-packaging] Missing packages for mbm-gpsd and mbm-gps-control To manage notifications about this bug go to: https://bugs.launchpad.net/modemmanager/+bug/783220/+subscriptions -- ubuntu-bugs mailing list ubuntu-bugs@lists.ubuntu.com https://lists.ubuntu.com/mailman/listinfo/ubuntu-bugs