From: Denis Kenzior <denkenz@gmail.com>
To: ofono@ofono.org
Subject: Re: [PATCH 3/8] sierramodem: Add ERI parsing functions
Date: Sun, 30 Dec 2012 20:19:11 -0600 [thread overview]
Message-ID: <50E0F61F.7080807@gmail.com> (raw)
In-Reply-To: <20121228193508.GI18339@alittletooquiet.net>
[-- Attachment #1: Type: text/plain, Size: 11534 bytes --]
Hi Forest,
On 12/28/2012 01:35 PM, Forest Bond wrote:
> From: Forest Bond<forest.bond@rapidrollout.com>
>
> These were borrowed from ModemManager.
> ---
> Makefile.am | 2 +
> drivers/sierramodem/eri.c | 282 +++++++++++++++++++++++++++++++++++++++++++++
> drivers/sierramodem/eri.h | 23 ++++
> 3 files changed, 307 insertions(+), 0 deletions(-)
> create mode 100644 drivers/sierramodem/eri.c
> create mode 100644 drivers/sierramodem/eri.h
>
> diff --git a/Makefile.am b/Makefile.am
> index 074326c..f799874 100644
> --- a/Makefile.am
> +++ b/Makefile.am
> @@ -321,6 +321,8 @@ builtin_modules += sierramodem
> builtin_sources += drivers/atmodem/atutil.h \
> drivers/sierramodem/sierramodem.h \
> drivers/sierramodem/sierramodem.c \
> + drivers/sierramodem/eri.h \
> + drivers/sierramodem/eri.c \
> drivers/sierramodem/cdma-netreg.c
>
> if PHONESIM
> diff --git a/drivers/sierramodem/eri.c b/drivers/sierramodem/eri.c
> new file mode 100644
> index 0000000..0a7929f
> --- /dev/null
> +++ b/drivers/sierramodem/eri.c
> @@ -0,0 +1,282 @@
> +/*
> + *
> + * oFono - Open Source Telephony
> + *
> + * Copyright (C) 2008-2009 Novell, Inc.
> + * Copyright (C) 2009-2012 Red Hat, Inc.
> + * Copyright (C) 2011-2012 Google, Inc.
> + * Copyright (C) 2012 Outpost Embedded, LLC. All rights reserved.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program; if not, write to the Free Software
> + * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA
> + *
> + */
> +
> +#ifdef HAVE_CONFIG_H
> +#include<config.h>
> +#endif
> +
> +#define _GNU_SOURCE
> +#include<glib.h>
> +#include<errno.h>
> +#include<stdlib.h>
> +
> +/* NOTE: The following functions originated in ModemManager. */
> +
Will be a bit pedantic here...
> +static gboolean get_uint_from_str(const gchar *str, guint *out)
> +{
> + gulong num;
> +
> + if (!str || !str[0])
> + return FALSE;
> +
> + for (num = 0; str[num]; num++) {
> + if (!g_ascii_isdigit(str[num]))
> + return FALSE;
> + }
This is unneeded if you use the 2nd parameter of strtoul properly.
> +
> + errno = 0;
> + num = strtoul(str, NULL, 10);
Need an empty line here
> + if (!errno&& num<= G_MAXUINT) {
> + *out = (guint)num;
> + return TRUE;
> + }
and here
> + return FALSE;
> +}
> +
> +typedef struct {
> + gint num;
> + gboolean roam_ind;
> + const gchar *banner;
> +} EriItem;
> +
Our preference is not to use typedefs and no CamelCase in struct / enum
definitions. The only exception is in G-lib style libraries, which this
one isn't.
> +/* NOTE: these may be Sprint-specific for now... */
> +static const EriItem eris[] = {
> + { 0, TRUE, "Digital or Analog Roaming" },
> + { 1, FALSE, "Home" },
> + { 2, TRUE, "Digital or Analog Roaming" },
> + { 3, TRUE, "Out of neighborhood" },
> + { 4, TRUE, "Out of building" },
> + { 5, TRUE, "Preferred system" },
> + { 6, TRUE, "Available System" },
> + { 7, TRUE, "Alliance Partner" },
> + { 8, TRUE, "Premium Partner" },
> + { 9, TRUE, "Full Service Functionality" },
> + { 10, TRUE, "Partial Service Functionality" },
> + { 64, TRUE, "Preferred system" },
> + { 65, TRUE, "Available System" },
> + { 66, TRUE, "Alliance Partner" },
> + { 67, TRUE, "Premium Partner" },
> + { 68, TRUE, "Full Service Functionality" },
> + { 69, TRUE, "Partial Service Functionality" },
> + { 70, TRUE, "Analog A" },
> + { 71, TRUE, "Analog B" },
> + { 72, TRUE, "CDMA 800 A" },
> + { 73, TRUE, "CDMA 800 B" },
> + { 74, TRUE, "International Roaming" },
> + { 75, TRUE, "Extended Network" },
> + { 76, FALSE, "Campus" },
> + { 77, FALSE, "In Building" },
> + { 78, TRUE, "Regional" },
> + { 79, TRUE, "Community" },
> + { 80, TRUE, "Business" },
> + { 81, TRUE, "Zone 1" },
> + { 82, TRUE, "Zone 2" },
> + { 83, TRUE, "National" },
> + { 84, TRUE, "Local" },
> + { 85, TRUE, "City" },
> + { 86, TRUE, "Government" },
> + { 87, TRUE, "USA" },
> + { 88, TRUE, "State" },
> + { 89, TRUE, "Resort" },
> + { 90, TRUE, "Headquarters" },
> + { 91, TRUE, "Personal" },
> + { 92, FALSE, "Home" },
> + { 93, TRUE, "Residential" },
> + { 94, TRUE, "University" },
> + { 95, TRUE, "College" },
> + { 96, TRUE, "Hotel Guest" },
> + { 97, TRUE, "Rental" },
> + { 98, FALSE, "Corporate" },
> + { 99, FALSE, "Home Provider" },
> + { 100, FALSE, "Campus" },
> + { 101, FALSE, "In Building" },
> + { 102, TRUE, "Regional" },
> + { 103, TRUE, "Community" },
> + { 104, TRUE, "Business" },
> + { 105, TRUE, "Zone 1" },
> + { 106, TRUE, "Zone 2" },
> + { 107, TRUE, "National" },
> + { 108, TRUE, "Local" },
> + { 109, TRUE, "City" },
> + { 110, TRUE, "Government" },
> + { 111, TRUE, "USA" },
> + { 112, TRUE, "State" },
> + { 113, TRUE, "Resort" },
> + { 114, TRUE, "Headquarters" },
> + { 115, TRUE, "Personal" },
> + { 116, FALSE, "Home" },
> + { 117, TRUE, "Residential" },
> + { 118, TRUE, "University" },
> + { 119, TRUE, "College" },
> + { 120, TRUE, "Hotel Guest" },
> + { 121, TRUE, "Rental" },
> + { 122, FALSE, "Corporate" },
> + { 123, FALSE, "Home Provider" },
> + { 124, TRUE, "International" },
> + { 125, TRUE, "International" },
> + { 126, TRUE, "International" },
> + { 127, FALSE, "Premium Service" },
> + { 128, FALSE, "Enhanced Service" },
> + { 129, FALSE, "Enhanced Digital" },
> + { 130, FALSE, "Enhanced Roaming" },
> + { 131, FALSE, "Alliance Service" },
> + { 132, FALSE, "Alliance Network" },
> + { 133, FALSE, "Data Roaming" }, /* Sprint: Vision Roaming */
> + { 134, FALSE, "Extended Service" },
> + { 135, FALSE, "Expanded Services" },
> + { 136, FALSE, "Expanded Network" },
> + { 137, TRUE, "Premium Service" },
> + { 138, TRUE, "Enhanced Service" },
> + { 139, TRUE, "Enhanced Digital" },
> + { 140, TRUE, "Enhanced Roaming" },
> + { 141, TRUE, "Alliance Service" },
> + { 142, TRUE, "Alliance Network" },
> + { 143, TRUE, "Data Roaming" }, /* Sprint: Vision Roaming */
> + { 144, TRUE, "Extended Service" },
> + { 145, TRUE, "Expanded Services" },
> + { 146, TRUE, "Expanded Network" },
> + { 147, TRUE, "Premium Service" },
> + { 148, TRUE, "Enhanced Service" },
> + { 149, TRUE, "Enhanced Digital" },
> + { 150, TRUE, "Enhanced Roaming" },
> + { 151, TRUE, "Alliance Service" },
> + { 152, TRUE, "Alliance Network" },
> + { 153, TRUE, "Data Roaming" }, /* Sprint: Vision Roaming */
> + { 154, TRUE, "Extended Service" },
> + { 155, TRUE, "Expanded Services" },
> + { 156, TRUE, "Expanded Network" },
> + { 157, TRUE, "Premium International" },
> + { 158, TRUE, "Premium International" },
> + { 159, TRUE, "Premium International" },
> + { 160, TRUE, NULL },
> + { 161, TRUE, NULL },
> + { 162, FALSE, NULL },
> + { 163, FALSE, NULL },
> + { 164, FALSE, "Extended Voice/Data Network" },
> + { 165, FALSE, "Extended Voice/Data Network" },
> + { 166, TRUE, "Extended Voice/Data Network" },
> + { 167, FALSE, "Extended Broadband" },
> + { 168, FALSE, "Extended Broadband" },
> + { 169, TRUE, "Extended Broadband" },
> + { 170, FALSE, "Extended Data" },
> + { 171, FALSE, "Extended Data" },
> + { 172, TRUE, "Extended Data" },
> + { 173, FALSE, "Extended Data Network" },
> + { 174, FALSE, "Extended Data Network" },
> + { 175, TRUE, "Extended Data Network" },
> + { 176, FALSE, "Extended Network" },
> + { 177, FALSE, "Extended Network" },
> + { 178, TRUE, "Extended Network" },
> + { 179, FALSE, "Extended Service" },
> + { 180, TRUE, "Extended Service" },
> + { 181, FALSE, "Extended Voice" },
> + { 182, FALSE, "Extended Voice" },
> + { 183, TRUE, "Extended Voice" },
> + { 184, FALSE, "Extended Voice/Data" },
> + { 185, FALSE, "Extended Voice/Data" },
> + { 186, TRUE, "Extended Voice/Data" },
> + { 187, FALSE, "Extended Voice Network" },
> + { 188, FALSE, "Extended Voice Network" },
> + { 189, TRUE, "Extended Voice Network" },
> + { 190, FALSE, "Extended Voice/Data" },
> + { 191, FALSE, "Extended Voice/Data" },
> + { 192, TRUE, "Extended Voice/Data" },
> + { 193, TRUE, "International" },
> + { 194, FALSE, "International Services" },
> + { 195, FALSE, "International Voice" },
> + { 196, FALSE, "International Voice/Data" },
> + { 197, FALSE, "International Voice/Data" },
> + { 198, TRUE, "International Voice/Data" },
> + { 199, FALSE, "Extended Voice/Data Network" },
> + { 200, TRUE, "Extended Voice/Data Network" },
> + { 201, TRUE, "Extended Voice/Data Network" },
> + { 202, FALSE, "Extended Broadband" },
> + { 203, TRUE, "Extended Broadband" },
> + { 204, TRUE, "Extended Broadband" },
> + { 205, FALSE, "Extended Data" },
> + { 206, TRUE, "Extended Data" },
> + { 207, TRUE, "Extended Data" },
> + { 208, FALSE, "Extended Data Network" },
> + { 209, TRUE, "Extended Data Network" },
> + { 210, TRUE, "Extended Data Network" },
> + { 211, FALSE, "Extended Network" },
> + { 212, TRUE, "Extended Network" },
> + { 213, FALSE, "Extended Service" },
> + { 214, TRUE, "Extended Service" },
> + { 215, TRUE, "Extended Service" },
> + { 216, FALSE, "Extended Voice" },
> + { 217, TRUE, "Extended Voice" },
> + { 218, TRUE, "Extended Voice" },
> + { 219, FALSE, "Extended Voice/Data" },
> + { 220, TRUE, "Extended Voice/Data" },
> + { 221, TRUE, "Extended Voice/Data" },
> + { 222, FALSE, "Extended Voice Network" },
> + { 223, FALSE, "Extended Voice Network" },
> + { 224, TRUE, "Extended Voice Network" },
> + { 225, FALSE, "Extended Voice/Data" },
> + { 226, TRUE, "Extended Voice/Data" },
> + { 227, TRUE, "Extended Voice/Data" },
> + { 228, TRUE, "International" },
> + { 229, TRUE, "International" },
> + { 230, TRUE, "International Services" },
> + { 231, TRUE, "International Voice" },
> + { 232, FALSE, "International Voice/Data" },
> + { 233, TRUE, "International Voice/Data" },
> + { 234, TRUE, "International Voice/Data" },
> + { 235, TRUE, "Premium International" },
> + { 236, TRUE, NULL },
> + { 237, TRUE, NULL },
> + { 238, FALSE, NULL },
> + { 239, FALSE, NULL },
> + { -1, FALSE, NULL },
> +};
> +
> +gboolean sierramodem_parse_eri(const gchar *reply, gboolean *out_roaming,
> + guint *out_ind, const gchar **out_desc)
> +{
> + guint ind;
> + const EriItem *iter =&eris[0];
> + gboolean found = FALSE;
> +
> + g_return_val_if_fail(reply != NULL, FALSE);
> + g_return_val_if_fail(out_roaming != NULL, FALSE);
> +
> + if (get_uint_from_str(reply,&ind)) {
> + if (out_ind)
> + *out_ind = ind;
> +
> + while (iter->num != -1) {
> + if (iter->num == ind) {
> + *out_roaming = iter->roam_ind;
> + if (out_desc)
> + *out_desc = iter->banner;
> + found = TRUE;
> + break;
> + }
> + iter++;
> + }
> + }
> +
> + return found;
In general this is not how we structure code like this. Preferred way
would be something like:
while () {
if (iter->num != ind)
continue;
blah;
return TRUE;
}
return FALSE;
> +}
Regards,
-Denis
next prev parent reply other threads:[~2012-12-31 2:19 UTC|newest]
Thread overview: 23+ messages / expand[flat|nested] mbox.gz Atom feed top
2012-12-28 19:31 [PATCH 0/8] Sierra Wireless CDMA modem support Forest Bond
2012-12-28 19:34 ` [PATCH 1/8] sierramodem: Add skeleton for Sierra Wireless modem driver Forest Bond
2012-12-28 19:34 ` [PATCH 2/8] sierramodem: Add skeleton cdma netreg driver Forest Bond
2012-12-31 2:11 ` Denis Kenzior
2012-12-28 19:35 ` [PATCH 3/8] sierramodem: Add ERI parsing functions Forest Bond
2012-12-31 2:19 ` Denis Kenzior [this message]
2012-12-28 19:35 ` [PATCH 4/8] sierramodem: Report network registration status Forest Bond
2012-12-31 2:26 ` Denis Kenzior
2012-12-28 19:35 ` [PATCH 5/8] sierra: Create GPRS context in post_sim function Forest Bond
2012-12-31 2:39 ` Denis Kenzior
2012-12-28 19:36 ` [PATCH 6/8] sierra: Initialize GSM error reporting separately Forest Bond
2012-12-31 2:40 ` Denis Kenzior
2012-12-28 19:36 ` [PATCH 7/8] sierra: Support CDMA modems Forest Bond
2012-12-31 2:34 ` Denis Kenzior
2013-03-04 16:48 ` Forest Bond
2013-03-04 23:03 ` Denis Kenzior
2013-03-05 19:36 ` Forest Bond
2013-03-05 20:52 ` Denis Kenzior
2013-03-05 21:45 ` =?unknown-8bit?q?Bj=C3=B8rn?= Mork
2013-03-05 22:15 ` Forest Bond
2013-03-06 9:43 ` =?unknown-8bit?q?Bj=C3=B8rn?= Mork
2013-03-06 13:36 ` Forest Bond
2012-12-28 19:36 ` [PATCH 8/8] udevng: Support single-interface sierra devices Forest Bond
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=50E0F61F.7080807@gmail.com \
--to=denkenz@gmail.com \
--cc=ofono@ofono.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox