[PATCH] [Telit plugin] Add load_current_bands interface
Aleksander Morgado
aleksander at aleksander.es
Thu Feb 11 08:04:53 UTC 2016
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
More information about the ModemManager-devel
mailing list