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
_______________________________________________
ofono mailing list
ofono@ofono.org
http://lists.ofono.org/listinfo/ofono
