Re: [PATCH 09/12] atmodem: add gnss driver
Hi Jarko,
On 03/24/2011 08:46 AM, Jarko Poutiainen wrote:
---
Makefile.am | 3 +-
drivers/atmodem/atmodem.c | 2 +
drivers/atmodem/atmodem.h | 3 +
drivers/atmodem/gnss.c | 282 +++++++++++++++++++++++++++++++++++++++++++++
4 files changed, 289 insertions(+), 1 deletions(-)
create mode 100644 drivers/atmodem/gnss.c
I applied this patch, however:
+static gboolean gnss_parse_report(GAtResult *result, const char
*prefix,
+ const char **xml)
+{
+ GAtResultIter iter;
+
+ g_at_result_iter_init(&iter, result);
+
+ if (!g_at_result_iter_next(&iter, prefix))
+ return FALSE;
+
+ if (!g_at_result_iter_next_unquoted_string(&iter, xml))
+ return FALSE;
+
+ return TRUE;
+}
+
+static void gnss_report(GAtResult *result, gpointer user_data)
+{
+ struct ofono_gnss *gnss = user_data;
+ const char *xml;
+
+ DBG("");
+
+ xml = NULL;
+ if (!gnss_parse_report(result, "+CPOSR:", &xml)) {
+ ofono_error("Unable to parse CPOSR notification");
+ return;
+ }
+
+ if (xml == NULL) {
+ ofono_error("Unable to parse CPOSR notification");
+ return;
+ }
+
+ ofono_gnss_notify_posr_request(gnss, xml);
+}
The implementation of CPOSR is pretty much unacceptable. You're relying
on the agent to parse the XML piecemeal. I don't like this at all. It
creates unnecessary round-trips over D-Bus for each CPOSR we receive,
not to mention ambiguity in exceptional conditions (e.g. a modem reset
happens during CPOSR emission).
I'd like you to modify the driver to detect the start and end of CPOSR
strings, assemble the XML fragments into a single chunk and only call
ofono_gnss_notify_posr_request once the XML string is assembled.
Regards,
-Denis