[PATCH] [Telit plugin] Add load_current_bands interface
c.lobrano at gmail.com
c.lobrano at gmail.com
Thu Feb 11 09:32:50 UTC 2016
Thank you Aleksander,
I agree with your comments, I will provide an updated version soon
Thanks again,
Carlo
On gio, feb 11, 2016 at 9:04 , Aleksander Morgado
<aleksander at aleksander.es> wrote:
> Hey Carlo,
>
> Sorry for being so late reviewing. See some minor comments below.
>
> Also, looks like the patch didn't apply on top of git master, likely
> due
> to the recent patches from Daniele that got merged earlier.
>
> Looks very good, though :)
>
>
> On 28/01/16 17:30, c.lobrano at gmail.com wrote:
>> From: Carlo Lobrano <c.lobrano at gmail.com>
>>
>> ---
>> plugins/telit/mm-broadband-modem-telit.c | 78
>> +++++++++++++-----
>> plugins/telit/mm-modem-helpers-telit.c | 97
>> +++++++++++++++--------
>> plugins/telit/mm-modem-helpers-telit.h | 21 +++--
>> plugins/telit/tests/test-mm-modem-helpers-telit.c | 89
>> +++++++++++++++++++--
>> 4 files changed, 220 insertions(+), 65 deletions(-)
>>
>> diff --git a/plugins/telit/mm-broadband-modem-telit.c
>> b/plugins/telit/mm-broadband-modem-telit.c
>> index 77b1800..bb2c3ce 100644
>> --- a/plugins/telit/mm-broadband-modem-telit.c
>> +++ b/plugins/telit/mm-broadband-modem-telit.c
>> @@ -41,28 +41,30 @@ G_DEFINE_TYPE_EXTENDED (MMBroadbandModemTelit,
>> mm_broadband_modem_telit, MM_TYPE
>> G_IMPLEMENT_INTERFACE
>> (MM_TYPE_IFACE_MODEM_3GPP, iface_modem_3gpp_init));
>>
>>
>> /*****************************************************************************/
>> -/* Load supported bands (Modem interface) */
>> +/* Load current bands (Modem interface) */
>> +
>> typedef struct {
>> MMIfaceModem *self;
>> GSimpleAsyncResult *result;
>> gboolean mm_modem_is_2g;
>> gboolean mm_modem_is_3g;
>> gboolean mm_modem_is_4g;
>> -} LoadSupportedBandsContext;
>> + LoadBandsType band_type;
>> +} LoadBandsContext;
>>
>> static void
>> -load_supported_bands_context_complete_and_free
>> (LoadSupportedBandsContext *ctx)
>> +load_supported_bands_context_complete_and_free (LoadBandsContext
>> *ctx)
>> {
>
> This should be renamed as "load_bands_context_complete_and_free" I
> guess.
>
>> g_simple_async_result_complete (ctx->result);
>> g_object_unref (ctx->result);
>> g_object_unref (ctx->self);
>> - g_slice_free (LoadSupportedBandsContext, ctx);
>> + g_slice_free (LoadBandsContext, ctx);
>> }
>>
>> static GArray *
>> -modem_load_supported_bands_finish (MMIfaceModem *self,
>> - GAsyncResult *res,
>> - GError **error)
>> +modem_load_bands_finish (MMIfaceModem *self,
>> + GAsyncResult *res,
>> + GError **error)
>> {
>> if (g_simple_async_result_propagate_error
>> (G_SIMPLE_ASYNC_RESULT (res), error))
>> return NULL;
>> @@ -72,9 +74,9 @@ modem_load_supported_bands_finish (MMIfaceModem
>> *self,
>> }
>>
>> static void
>> -load_supported_bands_ready (MMBaseModem *self,
>> - GAsyncResult *res,
>> - LoadSupportedBandsContext *ctx)
>> +load_bands_ready (MMBaseModem *self,
>> + GAsyncResult *res,
>> + LoadBandsContext *ctx)
>> {
>> const gchar *response;
>> GError *error = NULL;
>> @@ -84,12 +86,13 @@ load_supported_bands_ready (MMBaseModem *self,
>>
>> if (!response)
>> g_simple_async_result_take_error (ctx->result, error);
>> - else if (!mm_telit_parse_supported_bands_response (response,
>> -
>> ctx->mm_modem_is_2g,
>> -
>> ctx->mm_modem_is_3g,
>> -
>> ctx->mm_modem_is_4g,
>> - &bands,
>> - &error))
>> + else if (!mm_telit_parse_bnd_response (response,
>> + ctx->mm_modem_is_2g,
>> + ctx->mm_modem_is_3g,
>> + ctx->mm_modem_is_4g,
>> + ctx->band_type,
>> + &bands,
>> + &error))
>> g_simple_async_result_take_error (ctx->result, error);
>> else
>> g_simple_async_result_set_op_res_gpointer (ctx->result,
>> bands, (GDestroyNotify)g_array_unref);
>> @@ -98,18 +101,51 @@ load_supported_bands_ready (MMBaseModem *self,
>> }
>>
>> static void
>> +modem_load_current_bands (MMIfaceModem *self,
>> + GAsyncReadyCallback callback,
>> + gpointer user_data)
>> +{
>> + LoadBandsContext *ctx;
>> +
>> + ctx = g_slice_new0 (LoadBandsContext);
>> +
>> + ctx->self = g_object_ref (self);
>> + ctx->mm_modem_is_2g = mm_iface_modem_is_2g (ctx->self);
>> + ctx->mm_modem_is_3g = mm_iface_modem_is_3g (ctx->self);
>> + ctx->mm_modem_is_4g = mm_iface_modem_is_4g (ctx->self);
>> + ctx->band_type = LOAD_CURRENT_BANDS;
>> +
>> + ctx->result = g_simple_async_result_new (G_OBJECT (self),
>> + callback,
>> + user_data,
>> +
>> modem_load_current_bands);
>> +
>> + mm_base_modem_at_command (MM_BASE_MODEM (self),
>> + "#BND?",
>> + 3,
>> + FALSE,
>> + (GAsyncReadyCallback)
>> load_bands_ready,
>> + ctx);
>> +}
>> +
>> +
>>
>> +/*****************************************************************************/
>> +/* Load supported bands (Modem interface) */
>> +
>> +static void
>> modem_load_supported_bands (MMIfaceModem *self,
>> GAsyncReadyCallback callback,
>> gpointer user_data)
>> {
>> - LoadSupportedBandsContext *ctx;
>> + LoadBandsContext *ctx;
>>
>> - ctx = g_slice_new0 (LoadSupportedBandsContext);
>> + ctx = g_slice_new0 (LoadBandsContext);
>>
>> ctx->self = g_object_ref (self);
>> ctx->mm_modem_is_2g = mm_iface_modem_is_2g (ctx->self);
>> ctx->mm_modem_is_3g = mm_iface_modem_is_3g (ctx->self);
>> ctx->mm_modem_is_4g = mm_iface_modem_is_4g (ctx->self);
>> + ctx->band_type = LOAD_SUPPORTED_BANDS;
>>
>> ctx->result = g_simple_async_result_new (G_OBJECT (self),
>> callback,
>> @@ -120,7 +156,7 @@ modem_load_supported_bands (MMIfaceModem *self,
>> "#BND=?",
>> 3,
>> FALSE,
>> - (GAsyncReadyCallback)
>> load_supported_bands_ready,
>> + (GAsyncReadyCallback)
>> load_bands_ready,
>> ctx);
>> }
>>
>> @@ -601,8 +637,10 @@ mm_broadband_modem_telit_init
>> (MMBroadbandModemTelit *self)
>> static void
>> iface_modem_init (MMIfaceModem *iface)
>> {
>> + iface->load_current_bands = modem_load_current_bands;
>> + iface->load_current_bands_finish = modem_load_bands_finish;
>> iface->load_supported_bands = modem_load_supported_bands;
>> - iface->load_supported_bands_finish =
>> modem_load_supported_bands_finish;
>> + iface->load_supported_bands_finish = modem_load_bands_finish;
>> iface->load_unlock_retries_finish =
>> modem_load_unlock_retries_finish;
>> iface->load_unlock_retries = modem_load_unlock_retries;
>> iface->reset = modem_reset;
>> diff --git a/plugins/telit/mm-modem-helpers-telit.c
>> b/plugins/telit/mm-modem-helpers-telit.c
>> index 91609f2..0bd44b8 100644
>> --- a/plugins/telit/mm-modem-helpers-telit.c
>> +++ b/plugins/telit/mm-modem-helpers-telit.c
>> @@ -77,12 +77,16 @@ mm_telit_parse_csim_response (const guint step,
>> return retries;
>> }
>>
>> +#define SUPP_BAND_RESPONSE_REGEX
>> "#BND:\\s*\\((?P<Bands2G>.*)\\),\\s*\\((?P<Bands3G>.*)\\)"
>> +#define SUPP_BAND_4G_MODEM_RESPONSE_REGEX
>> "#BND:\\s*\\((?P<Bands2G>.*)\\),\\s*\\((?P<Bands3G>.*)\\),\\s*\\((?P<Bands4G>\\d+-\\d+)\\)"
>> +#define CURR_BAND_RESPONSE_REGEX
>> "#BND:\\s*(?P<Bands2G>\\d+),\\s*(?P<Bands3G>\\d+)"
>> +#define CURR_BAND_4G_MODEM_RESPONSE_REGEX
>> "#BND:\\s*(?P<Bands2G>\\d+),\\s*(?P<Bands3G>\\d+),\\s*(?P<Bands4G>\\d+)"
>> +
>>
>> /*****************************************************************************/
>> -/* #BND=? response parser
>> +/* #BND response parser
>> *
>> - * Example:
>> - * AT#BND=?
>> - * #BND: <2G band flags>,<3G band flags>[, <4G band flags>]
>> + * AT#BND=?
>> + * #BND: <2G band flags range>,<3G band flags range>[, <4G
>> band flags range>]
>> *
>> * where "band flags" is a list of numbers definining the
>> supported bands.
>> * Note that the one Telit band flag may represent more than one
>> MM band.
>> @@ -113,29 +117,55 @@ mm_telit_parse_csim_response (const guint
>> step,
>> * (2-4106)
>> * 2 = 2^1 --> lower supported band B2
>> * 4106 = 2^1 + 2^3 + 2^12 --> the supported bands are B2,
>> B4, B13
>> + *
>> + *
>> + * AT#BND?
>> + * #BND: <2G band flags>,<3G band flags>[, <4G band flags>]
>> + *
>> + * where "band flags" is a number definining the current bands.
>> + * Note that the one Telit band flag may represent more than one
>> MM band.
>> + *
>> + * e.g.
>> + *
>> + * #BND: 0,4
>> + *
>> + * 0 = 2G band flag 0 is EGSM + DCS
>> + * 4 = 3G band flag 4 is U1900 + U850
>> + *
>> */
>> -
>> -#define SUPP_BAND_RESPONSE_REGEX
>> "#BND:\\s*\\((?P<Bands2G>.*)\\),\\s*\\((?P<Bands3G>.*)\\)"
>> -#define SUPP_BAND_4G_MODEM_RESPONSE_REGEX
>> "#BND:\\s*\\((?P<Bands2G>.*)\\),\\s*\\((?P<Bands3G>.*)\\),\\s*\\((?P<Bands4G>\\d+-\\d+)\\)"
>> -
>> gboolean
>> -mm_telit_parse_supported_bands_response (const gchar *response,
>> - const gboolean
>> modem_is_2g,
>> - const gboolean
>> modem_is_3g,
>> - const gboolean
>> modem_is_4g,
>> - GArray **supported_bands,
>> - GError **error)
>> +mm_telit_parse_bnd_response (const gchar *response,
>> + const gboolean modem_is_2g,
>> + const gboolean modem_is_3g,
>> + const gboolean modem_is_4g,
>> + const LoadBandsType band_type,
>
> For non-pointers we don't really use "const" in method calls; having
> "const" there doesn't harm, but doesn't really do any good either, so
> no
> big deal.
>
>> + GArray **supported_bands,
>> + GError **error)
>> {
>> GArray *bands = NULL;
>> GMatchInfo *match_info = NULL;
>> GRegex *r = NULL;
>> gboolean ret = FALSE;
>>
>> - /* Parse #BND=? response */
>> - if (modem_is_4g)
>> - r = g_regex_new (SUPP_BAND_4G_MODEM_RESPONSE_REGEX,
>> G_REGEX_RAW, 0, NULL);
>> - else
>> - r = g_regex_new (SUPP_BAND_RESPONSE_REGEX, G_REGEX_RAW, 0,
>> NULL);
>> + switch (band_type) {
>> + case LOAD_SUPPORTED_BANDS:
>> + /* Parse #BND=? response */
>> + if (modem_is_4g)
>> + r = g_regex_new
>> (SUPP_BAND_4G_MODEM_RESPONSE_REGEX, G_REGEX_RAW, 0, NULL);
>> + else
>> + r = g_regex_new (SUPP_BAND_RESPONSE_REGEX,
>> G_REGEX_RAW, 0, NULL);
>> + break;
>> + case LOAD_CURRENT_BANDS:
>> + /* Parse #BND? response */
>> + if (modem_is_4g)
>> + r = g_regex_new
>> (CURR_BAND_4G_MODEM_RESPONSE_REGEX, G_REGEX_RAW, 0, NULL);
>> + else
>> + r = g_regex_new (CURR_BAND_RESPONSE_REGEX,
>> G_REGEX_RAW, 0, NULL);
>> + break;
>> + default:
>> + break;
>> + }
>> +
>>
>> if (!g_regex_match (r, response, 0, &match_info)) {
>> g_set_error (error, MM_CORE_ERROR, MM_CORE_ERROR_FAILED,
>> @@ -310,7 +340,7 @@ mm_telit_get_4g_mm_bands(GMatchInfo *match_info,
>> gboolean ret = TRUE;
>> gchar *match_str = NULL;
>> guint i;
>> - guint max_value;
>> + guint value;
>> gchar **tokens;
>>
>> match_str = g_match_info_fetch_named (match_info, "Bands4G");
>> @@ -322,25 +352,28 @@ mm_telit_get_4g_mm_bands(GMatchInfo
>> *match_info,
>> goto end;
>> }
>>
>> - tokens = g_strsplit (match_str, "-", -1);
>> - if (tokens == NULL) {
>> - g_set_error (error, MM_CORE_ERROR, MM_CORE_ERROR_FAILED,
>> - "Could not get 4G band ranges from string
>> '%s'",
>> - match_str);
>> - ret = FALSE;
>> - goto end;
>> + if (strstr(match_str, "-")) {
>> + tokens = g_strsplit (match_str, "-", -1);
>> + if (tokens == NULL) {
>> + g_set_error (error, MM_CORE_ERROR,
>> MM_CORE_ERROR_FAILED,
>> + "Could not get 4G band ranges from string
>> '%s'",
>> + match_str);
>> + ret = FALSE;
>> + goto end;
>> + }
>> + sscanf (tokens[1], "%d", &value);
>> + } else {
>> + sscanf (match_str, "%d", &value);
>> }
>>
>> - sscanf (tokens[1], "%d", &max_value);
>>
>> - for (i = 0; max_value > 0; i++) {
>> - if (max_value % 2 != 0) {
>> + for (i = 0; value > 0; i++) {
>> + if (value % 2 != 0) {
>> band = MM_MODEM_BAND_EUTRAN_I + i;
>> g_array_append_val (*bands, band);
>> }
>> - max_value = max_value >> 1;
>> + value = value >> 1;
>> }
>> -
>> end:
>> if (match_str != NULL)
>> g_free (match_str);
>> diff --git a/plugins/telit/mm-modem-helpers-telit.h
>> b/plugins/telit/mm-modem-helpers-telit.h
>> index d0d74b5..affdf76 100644
>> --- a/plugins/telit/mm-modem-helpers-telit.h
>> +++ b/plugins/telit/mm-modem-helpers-telit.h
>> @@ -65,14 +65,21 @@ gint mm_telit_parse_csim_response (const guint
>> step,
>> const gchar *response,
>> GError **error);
>>
>> -/* #BND=? response parser */
>> +typedef enum {
>> + LOAD_SUPPORTED_BANDS,
>> + LOAD_CURRENT_BANDS
>> +} LoadBandsType;
>> +
>
> This enum could be prefixed with "MMTelit", so that we know in the
> implementation that it's Telit specific.
>
>> +/* #BND response parser */
>> gboolean
>> -mm_telit_parse_supported_bands_response (const gchar *response,
>> - const gboolean
>> modem_is_2g,
>> - const gboolean
>> modem_is_3g,
>> - const gboolean
>> modem_is_4g,
>> - GArray **supported_bands,
>> - GError **error);
>> +mm_telit_parse_bnd_response (const gchar *response,
>> + const gboolean modem_is_2g,
>> + const gboolean modem_is_3g,
>> + const gboolean modem_is_4g,
>> + const LoadBandsType band_type,
>> + GArray **supported_bands,
>> + GError **error);
>> +
>>
>> gboolean mm_telit_bands_contains (GArray *mm_bands, const
>> MMModemBand mm_band);
>>
>> diff --git a/plugins/telit/tests/test-mm-modem-helpers-telit.c
>> b/plugins/telit/tests/test-mm-modem-helpers-telit.c
>> index 857b93f..cac10fb 100644
>> --- a/plugins/telit/tests/test-mm-modem-helpers-telit.c
>> +++ b/plugins/telit/tests/test-mm-modem-helpers-telit.c
>> @@ -213,12 +213,13 @@ test_parse_supported_bands_response (void) {
>> GArray* bands = NULL;
>>
>> for (i = 0; supported_band_mapping_tests[i].response != NULL;
>> i++) {
>> - res = mm_telit_parse_supported_bands_response
>> (supported_band_mapping_tests[i].response,
>> -
>> supported_band_mapping_tests[i].modem_is_2g,
>> -
>> supported_band_mapping_tests[i].modem_is_3g,
>> -
>> supported_band_mapping_tests[i].modem_is_4g,
>> - &bands,
>> - &error);
>> + res = mm_telit_parse_bnd_response
>> (supported_band_mapping_tests[i].response,
>> +
>> supported_band_mapping_tests[i].modem_is_2g,
>> +
>> supported_band_mapping_tests[i].modem_is_3g,
>> +
>> supported_band_mapping_tests[i].modem_is_4g,
>> + LOAD_SUPPORTED_BANDS,
>> + &bands,
>> + &error);
>> g_assert_no_error (error);
>> g_assert_true (res);
>>
>> @@ -239,6 +240,81 @@ test_parse_supported_bands_response (void) {
>> }
>> }
>>
>> +
>> +static BNDResponseTest current_band_mapping_tests [] = {
>> + { "#BND: 0,5", TRUE, TRUE, FALSE, 3, { MM_MODEM_BAND_EGSM,
>> + MM_MODEM_BAND_DCS,
>> + MM_MODEM_BAND_U900
>> + }
>> + },
>> + { "#BND: 1,3", TRUE, TRUE, FALSE, 5, { MM_MODEM_BAND_EGSM,
>> + MM_MODEM_BAND_PCS,
>> + MM_MODEM_BAND_U2100,
>> + MM_MODEM_BAND_U1900,
>> + MM_MODEM_BAND_U850,
>> + }
>> + },
>> + { "#BND: 2,7", TRUE, TRUE, FALSE, 3, { MM_MODEM_BAND_DCS,
>> + MM_MODEM_BAND_G850,
>> + MM_MODEM_BAND_U17IV
>> + },
>> + },
>> + { "#BND: 3,0,1", TRUE, TRUE, TRUE, 4, { MM_MODEM_BAND_PCS,
>> + MM_MODEM_BAND_G850,
>> + MM_MODEM_BAND_U2100,
>> + MM_MODEM_BAND_EUTRAN_I
>> + }
>> + },
>> + { "#BND: 0,0,3", TRUE, FALSE, TRUE, 4, { MM_MODEM_BAND_EGSM,
>> + MM_MODEM_BAND_DCS,
>> + MM_MODEM_BAND_EUTRAN_I,
>> + MM_MODEM_BAND_EUTRAN_II
>> + }
>> + },
>> + { "#BND: 0,0,3", FALSE, FALSE, TRUE, 2, {
>> MM_MODEM_BAND_EUTRAN_I,
>> +
>> MM_MODEM_BAND_EUTRAN_II
>> + }
>> + },
>> + { NULL, FALSE, FALSE, FALSE, 0, {}},
>> +};
>> +
>> +static void
>> +test_parse_current_bands_response (void) {
>> + GError* error = NULL;
>> + gboolean res = FALSE;
>> + guint i, j;
>> + GArray* bands = NULL;
>> +
>> + for (i = 0; current_band_mapping_tests[i].response != NULL;
>> i++) {
>> + res = mm_telit_parse_bnd_response
>> (current_band_mapping_tests[i].response,
>> +
>> current_band_mapping_tests[i].modem_is_2g,
>> +
>> current_band_mapping_tests[i].modem_is_3g,
>> +
>> current_band_mapping_tests[i].modem_is_4g,
>> + LOAD_CURRENT_BANDS,
>> + &bands,
>> + &error);
>> + g_assert_no_error (error);
>> + g_assert_true (res);
>> +
>> +
>> + for (j = 0; j <
>> current_band_mapping_tests[i].mm_bands_len; j++) {
>> + MMModemBand ref;
>> + MMModemBand cur;
>> +
>> + ref = current_band_mapping_tests[i].mm_bands[j];
>> + cur = g_array_index (bands, MMModemBand, j);
>> + g_assert_cmpint (cur, ==, ref);
>> + }
>> +
>> + g_assert_cmpint (bands->len, ==,
>> current_band_mapping_tests[i].mm_bands_len);
>> +
>> + g_array_free (bands, FALSE);
>> + bands = NULL;
>> + }
>> +}
>> +
>> +
>> +
>> int main (int argc, char **argv)
>> {
>> setlocale (LC_ALL, "");
>> @@ -250,5 +326,6 @@ int main (int argc, char **argv)
>> g_test_add_func ("/MM/telit/bands/supported/bands_contains",
>> test_mm_bands_contains);
>> g_test_add_func ("/MM/telit/bands/supported/parse_band_flag",
>> test_parse_band_flag_str);
>> g_test_add_func
>> ("/MM/telit/bands/supported/parse_bands_response",
>> test_parse_supported_bands_response);
>> + g_test_add_func
>> ("/MM/telit/bands/current/parse_bands_response",
>> test_parse_current_bands_response);
>> return g_test_run ();
>> }
>>
>
>
> --
> Aleksander
> https://aleksander.es
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/modemmanager-devel/attachments/20160211/00f6ba96/attachment-0001.html>
More information about the ModemManager-devel
mailing list