Re: [PATCH 1/1] helpers: extend +WS46=? response parser to support ranges

[Daniele Palmas]

Hi Aleksander,

2017-06-16 10:39 GMT+02:00 Aleksander Morgado :
> Hey Daniele,
>
> On Fri, Jun 16, 2017 at 10:11 AM, Daniele Palmas  wrote:
>> Telit LTE modems could reply to +WS46=? with:
>>
>> +WS46: (12,22,25,28-31)
>>
>> This patch extends +WS46=? response parser to support ranges.
>> ---
>> Hi,
>>
>> current mm log is
>>
>> Jun 15 15:04:04 L2122 ModemManager[831]:  (ttyACM0): --> 
>> 'AT+WS46=?'
>> Jun 15 15:04:04 L2122 ModemManager[831]:  (ttyACM0): <-- ''
>> Jun 15 15:04:04 L2122 ModemManager[831]:  (ttyACM0): <-- '+WS46: 
>> (12,22,25,28-31)OK'
>> Jun 15 15:04:04 L2122 ModemManager[831]: Invalid +WS46 mode reported: 28-31
>>
>> Daniele
>
>
> How about using this method instead? It already supports parsing lists
> of numbers, including intervals:
>GArray *mm_parse_uint_list (const gchar  *str, GError **error)
>
> See:
>
> https://cgit.freedesktop.org/ModemManager/ModemManager/tree/src/mm-modem-helpers.c#n139
>

Sorry, I missed that :-( I will write a new patch.

Thanks,
Daniele

>> ---
>>  src/mm-modem-helpers.c | 58 
>> +++---
>>  src/tests/test-modem-helpers.c | 33 
>>  2 files changed, 76 insertions(+), 15 deletions(-)
>>
>> diff --git a/src/mm-modem-helpers.c b/src/mm-modem-helpers.c
>> index 73822e1..dbd3cf8 100644
>> --- a/src/mm-modem-helpers.c
>> +++ b/src/mm-modem-helpers.c
>> @@ -920,29 +920,57 @@ mm_3gpp_parse_ws46_test_response (const gchar  
>> *response,
>>  for (i = 0; split && split[i]; i++) {
>>  guint val;
>>  guint j;
>> +guint k;
>> +GArray *range_values = NULL;
>> +
>> +range_values = g_array_new (FALSE, FALSE, sizeof (guint));
>>
>>  if (!mm_get_uint_from_str (split[i], )) {
>> -g_warning ("Invalid +WS46 mode reported: %s", split[i]);
>> -continue;
>> +gchar **split_range;
>> +guint range_start;
>> +guint range_end;
>> +
>> +split_range = g_strsplit (split[i], "-", -1);
>> +
>> +if (!split_range ||
>> +!mm_get_uint_from_str (split_range[0], _start) ||
>> +!mm_get_uint_from_str (split_range[1], _end)) {
>> +g_warning ("Invalid +WS46 mode reported: %s", split[i]);
>> +g_strfreev (split_range);
>> +g_array_free (range_values, TRUE);
>> +continue;
>> +}
>> +
>> +for (k = range_start; k <= range_end; k++)
>> +g_array_append_val (range_values, k);
>> +g_strfreev (split_range);
>> +} else {
>> +g_array_append_val (range_values, val);
>>  }
>>
>> -for (j = 0; j < G_N_ELEMENTS (ws46_modes); j++) {
>> -if (ws46_modes[j].ws46 == val) {
>> -if (val != 25) {
>> -if (ws46_modes[j].mode & MM_MODEM_MODE_4G)
>> -supported_4g = TRUE;
>> -if (ws46_modes[j].mode & MM_MODEM_MODE_3G)
>> -supported_3g = TRUE;
>> -if (ws46_modes[j].mode & MM_MODEM_MODE_2G)
>> -supported_2g = TRUE;
>> +for (k = 0; k < range_values->len; k++) {
>> +val = g_array_index (range_values, guint, k);
>> +
>> +for (j = 0; j < G_N_ELEMENTS (ws46_modes); j++) {
>> +if (ws46_modes[j].ws46 == val) {
>> +if (val != 25) {
>> +if (ws46_modes[j].mode & MM_MODEM_MODE_4G)
>> +supported_4g = TRUE;
>> +if (ws46_modes[j].mode & MM_MODEM_MODE_3G)
>> +supported_3g = TRUE;
>> +if (ws46_modes[j].mode & MM_MODEM_MODE_2G)
>> +supported_2g = TRUE;
>> +}
>> +g_array_append_val (modes, ws46_modes[j].mode);
>> +break;
>>  }
>> -g_array_append_val (modes, ws46_modes[j].mode);
>> -break;
>>  }
>> +
>> +if (j == G_N_ELEMENTS (ws46_modes))
>> +g_warning ("Unknown +WS46 mode reported: %s", split[i]);
>>  }
>>
>> -if (j == G_N_ELEMENTS (ws46_modes))
>> -g_warning ("Unknown +WS46 mode reported: %s", split[i]);
>> +g_array_free (range_values, TRUE);
>>  }
>>
>>  g_strfreev (split);
>> diff --git a/src/tests/test-modem-helpers.c b/src/tests/test-modem-helpers.c
>> index 5928412..feb4fae 100644
>> --- a/src/tests/test-modem-helpers.c
>> +++ b/src/tests/test-modem-helpers.c
>> @@ -215,6 +215,37 @@ test_ws46_response_telit_le866 (void)
>>  test_ws46_response (str, expected, G_N_ELEMENTS (expected));
>>  }
>>
>> +static void
>> +test_ws46_response_range_1 (void)
>> +{
>> +static const MMModemMode expected[] = {
>> +MM_MODEM_MODE_4G,
>> +

Re: [PATCH 1/1] helpers: extend +WS46=? response parser to support ranges

[Aleksander Morgado]

Hey Daniele,

On Fri, Jun 16, 2017 at 10:11 AM, Daniele Palmas  wrote:
> Telit LTE modems could reply to +WS46=? with:
>
> +WS46: (12,22,25,28-31)
>
> This patch extends +WS46=? response parser to support ranges.
> ---
> Hi,
>
> current mm log is
>
> Jun 15 15:04:04 L2122 ModemManager[831]:  (ttyACM0): --> 
> 'AT+WS46=?'
> Jun 15 15:04:04 L2122 ModemManager[831]:  (ttyACM0): <-- ''
> Jun 15 15:04:04 L2122 ModemManager[831]:  (ttyACM0): <-- '+WS46: 
> (12,22,25,28-31)OK'
> Jun 15 15:04:04 L2122 ModemManager[831]: Invalid +WS46 mode reported: 28-31
>
> Daniele


How about using this method instead? It already supports parsing lists
of numbers, including intervals:
   GArray *mm_parse_uint_list (const gchar  *str, GError **error)

See:
   
https://cgit.freedesktop.org/ModemManager/ModemManager/tree/src/mm-modem-helpers.c#n139

> ---
>  src/mm-modem-helpers.c | 58 
> +++---
>  src/tests/test-modem-helpers.c | 33 
>  2 files changed, 76 insertions(+), 15 deletions(-)
>
> diff --git a/src/mm-modem-helpers.c b/src/mm-modem-helpers.c
> index 73822e1..dbd3cf8 100644
> --- a/src/mm-modem-helpers.c
> +++ b/src/mm-modem-helpers.c
> @@ -920,29 +920,57 @@ mm_3gpp_parse_ws46_test_response (const gchar  
> *response,
>  for (i = 0; split && split[i]; i++) {
>  guint val;
>  guint j;
> +guint k;
> +GArray *range_values = NULL;
> +
> +range_values = g_array_new (FALSE, FALSE, sizeof (guint));
>
>  if (!mm_get_uint_from_str (split[i], )) {
> -g_warning ("Invalid +WS46 mode reported: %s", split[i]);
> -continue;
> +gchar **split_range;
> +guint range_start;
> +guint range_end;
> +
> +split_range = g_strsplit (split[i], "-", -1);
> +
> +if (!split_range ||
> +!mm_get_uint_from_str (split_range[0], _start) ||
> +!mm_get_uint_from_str (split_range[1], _end)) {
> +g_warning ("Invalid +WS46 mode reported: %s", split[i]);
> +g_strfreev (split_range);
> +g_array_free (range_values, TRUE);
> +continue;
> +}
> +
> +for (k = range_start; k <= range_end; k++)
> +g_array_append_val (range_values, k);
> +g_strfreev (split_range);
> +} else {
> +g_array_append_val (range_values, val);
>  }
>
> -for (j = 0; j < G_N_ELEMENTS (ws46_modes); j++) {
> -if (ws46_modes[j].ws46 == val) {
> -if (val != 25) {
> -if (ws46_modes[j].mode & MM_MODEM_MODE_4G)
> -supported_4g = TRUE;
> -if (ws46_modes[j].mode & MM_MODEM_MODE_3G)
> -supported_3g = TRUE;
> -if (ws46_modes[j].mode & MM_MODEM_MODE_2G)
> -supported_2g = TRUE;
> +for (k = 0; k < range_values->len; k++) {
> +val = g_array_index (range_values, guint, k);
> +
> +for (j = 0; j < G_N_ELEMENTS (ws46_modes); j++) {
> +if (ws46_modes[j].ws46 == val) {
> +if (val != 25) {
> +if (ws46_modes[j].mode & MM_MODEM_MODE_4G)
> +supported_4g = TRUE;
> +if (ws46_modes[j].mode & MM_MODEM_MODE_3G)
> +supported_3g = TRUE;
> +if (ws46_modes[j].mode & MM_MODEM_MODE_2G)
> +supported_2g = TRUE;
> +}
> +g_array_append_val (modes, ws46_modes[j].mode);
> +break;
>  }
> -g_array_append_val (modes, ws46_modes[j].mode);
> -break;
>  }
> +
> +if (j == G_N_ELEMENTS (ws46_modes))
> +g_warning ("Unknown +WS46 mode reported: %s", split[i]);
>  }
>
> -if (j == G_N_ELEMENTS (ws46_modes))
> -g_warning ("Unknown +WS46 mode reported: %s", split[i]);
> +g_array_free (range_values, TRUE);
>  }
>
>  g_strfreev (split);
> diff --git a/src/tests/test-modem-helpers.c b/src/tests/test-modem-helpers.c
> index 5928412..feb4fae 100644
> --- a/src/tests/test-modem-helpers.c
> +++ b/src/tests/test-modem-helpers.c
> @@ -215,6 +215,37 @@ test_ws46_response_telit_le866 (void)
>  test_ws46_response (str, expected, G_N_ELEMENTS (expected));
>  }
>
> +static void
> +test_ws46_response_range_1 (void)
> +{
> +static const MMModemMode expected[] = {
> +MM_MODEM_MODE_4G,
> +MM_MODEM_MODE_2G | MM_MODEM_MODE_3G,
> +MM_MODEM_MODE_2G | MM_MODEM_MODE_4G,
> +MM_MODEM_MODE_3G | MM_MODEM_MODE_4G,
> +};
> +const gchar *str = "+WS46: (28-31)";
> +
> +test_ws46_response (str, expected, G_N_ELEMENTS (expected));
> +}
> +
> +static void
> 

[PATCH 1/1] helpers: extend +WS46=? response parser to support ranges

[Daniele Palmas]

Telit LTE modems could reply to +WS46=? with:

+WS46: (12,22,25,28-31)

This patch extends +WS46=? response parser to support ranges.
---
Hi,

current mm log is

Jun 15 15:04:04 L2122 ModemManager[831]:  (ttyACM0): --> 'AT+WS46=?'
Jun 15 15:04:04 L2122 ModemManager[831]:  (ttyACM0): <-- ''
Jun 15 15:04:04 L2122 ModemManager[831]:  (ttyACM0): <-- '+WS46: 
(12,22,25,28-31)OK'
Jun 15 15:04:04 L2122 ModemManager[831]: Invalid +WS46 mode reported: 28-31

Daniele
---
 src/mm-modem-helpers.c | 58 +++---
 src/tests/test-modem-helpers.c | 33 
 2 files changed, 76 insertions(+), 15 deletions(-)

diff --git a/src/mm-modem-helpers.c b/src/mm-modem-helpers.c
index 73822e1..dbd3cf8 100644
--- a/src/mm-modem-helpers.c
+++ b/src/mm-modem-helpers.c
@@ -920,29 +920,57 @@ mm_3gpp_parse_ws46_test_response (const gchar  *response,
 for (i = 0; split && split[i]; i++) {
 guint val;
 guint j;
+guint k;
+GArray *range_values = NULL;
+
+range_values = g_array_new (FALSE, FALSE, sizeof (guint));
 
 if (!mm_get_uint_from_str (split[i], )) {
-g_warning ("Invalid +WS46 mode reported: %s", split[i]);
-continue;
+gchar **split_range;
+guint range_start;
+guint range_end;
+
+split_range = g_strsplit (split[i], "-", -1);
+
+if (!split_range ||
+!mm_get_uint_from_str (split_range[0], _start) ||
+!mm_get_uint_from_str (split_range[1], _end)) {
+g_warning ("Invalid +WS46 mode reported: %s", split[i]);
+g_strfreev (split_range);
+g_array_free (range_values, TRUE);
+continue;
+}
+
+for (k = range_start; k <= range_end; k++)
+g_array_append_val (range_values, k);
+g_strfreev (split_range);
+} else {
+g_array_append_val (range_values, val);
 }
 
-for (j = 0; j < G_N_ELEMENTS (ws46_modes); j++) {
-if (ws46_modes[j].ws46 == val) {
-if (val != 25) {
-if (ws46_modes[j].mode & MM_MODEM_MODE_4G)
-supported_4g = TRUE;
-if (ws46_modes[j].mode & MM_MODEM_MODE_3G)
-supported_3g = TRUE;
-if (ws46_modes[j].mode & MM_MODEM_MODE_2G)
-supported_2g = TRUE;
+for (k = 0; k < range_values->len; k++) {
+val = g_array_index (range_values, guint, k);
+
+for (j = 0; j < G_N_ELEMENTS (ws46_modes); j++) {
+if (ws46_modes[j].ws46 == val) {
+if (val != 25) {
+if (ws46_modes[j].mode & MM_MODEM_MODE_4G)
+supported_4g = TRUE;
+if (ws46_modes[j].mode & MM_MODEM_MODE_3G)
+supported_3g = TRUE;
+if (ws46_modes[j].mode & MM_MODEM_MODE_2G)
+supported_2g = TRUE;
+}
+g_array_append_val (modes, ws46_modes[j].mode);
+break;
 }
-g_array_append_val (modes, ws46_modes[j].mode);
-break;
 }
+
+if (j == G_N_ELEMENTS (ws46_modes))
+g_warning ("Unknown +WS46 mode reported: %s", split[i]);
 }
 
-if (j == G_N_ELEMENTS (ws46_modes))
-g_warning ("Unknown +WS46 mode reported: %s", split[i]);
+g_array_free (range_values, TRUE);
 }
 
 g_strfreev (split);
diff --git a/src/tests/test-modem-helpers.c b/src/tests/test-modem-helpers.c
index 5928412..feb4fae 100644
--- a/src/tests/test-modem-helpers.c
+++ b/src/tests/test-modem-helpers.c
@@ -215,6 +215,37 @@ test_ws46_response_telit_le866 (void)
 test_ws46_response (str, expected, G_N_ELEMENTS (expected));
 }
 
+static void
+test_ws46_response_range_1 (void)
+{
+static const MMModemMode expected[] = {
+MM_MODEM_MODE_4G,
+MM_MODEM_MODE_2G | MM_MODEM_MODE_3G,
+MM_MODEM_MODE_2G | MM_MODEM_MODE_4G,
+MM_MODEM_MODE_3G | MM_MODEM_MODE_4G,
+};
+const gchar *str = "+WS46: (28-31)";
+
+test_ws46_response (str, expected, G_N_ELEMENTS (expected));
+}
+
+static void
+test_ws46_response_range_2 (void)
+{
+static const MMModemMode expected[] = {
+MM_MODEM_MODE_2G,
+MM_MODEM_MODE_3G,
+MM_MODEM_MODE_2G | MM_MODEM_MODE_3G | MM_MODEM_MODE_4G,
+MM_MODEM_MODE_4G,
+MM_MODEM_MODE_2G | MM_MODEM_MODE_3G,
+MM_MODEM_MODE_2G | MM_MODEM_MODE_4G,
+MM_MODEM_MODE_3G | MM_MODEM_MODE_4G,
+};
+const gchar *str = "+WS46: (12,22,25,28-31)";
+
+test_ws46_response (str, expected, G_N_ELEMENTS (expected));
+}
+
 /*/
 /* Test CMGL responses */
 
@@