From mboxrd@z Thu Jan 1 00:00:00 1970 Content-Type: multipart/mixed; boundary="===============2619564221995061714==" MIME-Version: 1.0 From: Marcel Holtmann Subject: Re: [PATCH_v2 2/3] cdmamodem: Create network registration atom. Date: Sat, 23 Jul 2011 16:59:34 +0200 Message-ID: <1311433175.21109.346.camel@aeonflux> In-Reply-To: <1311394170-7381-3-git-send-email-bertrand.aygon@intel.com> List-Id: To: ofono@ofono.org --===============2619564221995061714== Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable Hi Bertrand, > Makefile.am | 3 +- > drivers/cdmamodem/cdmamodem.c | 2 + > drivers/cdmamodem/cdmamodem.h | 5 + > drivers/cdmamodem/network-registration.c | 139 ++++++++++++++++++++++++= ++++++ > 4 files changed, 148 insertions(+), 1 deletions(-) > create mode 100644 drivers/cdmamodem/network-registration.c > = > diff --git a/Makefile.am b/Makefile.am > index 837d374..e00f73b 100644 > --- a/Makefile.am > +++ b/Makefile.am > @@ -269,7 +269,8 @@ builtin_sources +=3D drivers/cdmamodem/cdmamodem.h \ > drivers/cdmamodem/cdmamodem.c \ > drivers/cdmamodem/voicecall.c \ > drivers/cdmamodem/devinfo.c \ > - drivers/cdmamodem/connman.c > + drivers/cdmamodem/connman.c \ > + drivers/cdmamodem/network-registration.c > endif > = > builtin_modules +=3D g1 > diff --git a/drivers/cdmamodem/cdmamodem.c b/drivers/cdmamodem/cdmamodem.c > index 1b19a4a..60d5315 100644 > --- a/drivers/cdmamodem/cdmamodem.c > +++ b/drivers/cdmamodem/cdmamodem.c > @@ -37,6 +37,7 @@ static int cdmamodem_init(void) > cdma_voicecall_init(); > cdma_devinfo_init(); > cdma_connman_init(); > + cdma_netreg_init(); > = > return 0; > } > @@ -46,6 +47,7 @@ static void cdmamodem_exit(void) > cdma_voicecall_exit(); > cdma_devinfo_exit(); > cdma_connman_exit(); > + cdma_netreg_exit(); > } > = > OFONO_PLUGIN_DEFINE(cdmamodem, "CDMA AT modem driver", VERSION, > diff --git a/drivers/cdmamodem/cdmamodem.h b/drivers/cdmamodem/cdmamodem.h > index 90e2848..df69082 100644 > --- a/drivers/cdmamodem/cdmamodem.h > +++ b/drivers/cdmamodem/cdmamodem.h > @@ -23,7 +23,12 @@ > = > extern void cdma_voicecall_init(void); > extern void cdma_voicecall_exit(void); > + > extern void cdma_devinfo_init(void); > extern void cdma_devinfo_exit(void); > + > extern void cdma_connman_init(void); > extern void cdma_connman_exit(void); > + Please separate cleanup fixes out into separate patch. > +extern void cdma_netreg_init(void); > +extern void cdma_netreg_exit(void); > diff --git a/drivers/cdmamodem/network-registration.c b/drivers/cdmamodem= /network-registration.c > new file mode 100644 > index 0000000..7641e66 > --- /dev/null > +++ b/drivers/cdmamodem/network-registration.c > @@ -0,0 +1,139 @@ > +/* > + * > + * oFono - Open Source Telephony > + * > + * Copyright (C) 2008-2011 Intel Corporation. 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-130= 1 USA > + * > + */ > + > +#ifdef HAVE_CONFIG_H > +#include > +#endif > + > +#define _GNU_SOURCE > +#include > +#include > +#include > + > +#include > + > +#include > +#include > +#include > + > +#include "gatchat.h" > +#include "gatresult.h" > + > +#include "common.h" I know that you are re-using the GSM network registration constants here and thus need common.h. Denis, is this acceptable for you? > +#include "cdmamodem.h" > + > +static const char *sysinfo_prefix[] =3D { "^SYSINFO:", NULL }; > + > +struct netreg_data { > + GAtChat *chat; > + unsigned int vendor; You have no vendor quirks. So leave this out. > +}; > + > +static void sysinfo_cb(gboolean ok, GAtResult *result, gpointer user_dat= a) > +{ > + struct ofono_cdma_netreg *netreg =3D user_data; > + gint service_state; > + gint service_domain; > + gint roaming_state; > + int status; > + GAtResultIter iter; > + > + if (!ok) > + return; > + > + g_at_result_iter_init(&iter, result); > + > + if (!g_at_result_iter_next(&iter, "^SYSINFO:")) > + return; > + > + if (!g_at_result_iter_next_number(&iter, &service_state)) > + return; > + > + if (!g_at_result_iter_next_number(&iter, &service_domain)) > + return; > + > + if (!g_at_result_iter_next_number(&iter, &roaming_state)) > + return; > + > + switch (service_state) { > + case 0: > + case 4: Please put at least comments behind every case label so that we know what it does mean. > + status =3D NETWORK_REGISTRATION_STATUS_NOT_REGISTERED; > + break; > + case 2: > + if (roaming_state) > + status =3D NETWORK_REGISTRATION_STATUS_ROAMING; > + else > + status =3D NETWORK_REGISTRATION_STATUS_REGISTERED; > + break; > + default: > + status =3D NETWORK_REGISTRATION_STATUS_UNKNOWN; > + break; > + } > + > + ofono_cdma_netreg_register(netreg); > + > + ofono_cdma_netreg_status_notify(netreg, status); > +} How do we get unsolicited notification when the status changes? > + > +static int cdma_netreg_probe(struct ofono_cdma_netreg *netreg, > + unsigned int vendor, void *data) > +{ > + GAtChat *chat =3D data; > + struct netreg_data *nd; > + > + nd =3D g_new0(struct netreg_data, 1); Use g_try_new0 here. Even while old code is not doing this, we try to update it eventually. > + > + nd->chat =3D g_at_chat_clone(chat); > + nd->vendor =3D vendor; > + ofono_cdma_netreg_set_data(netreg, nd); > + > + g_at_chat_send(nd->chat, "AT^SYSINFO", sysinfo_prefix, > + sysinfo_cb, netreg, NULL); > + > + return 0; > +} > + > +static void cdma_netreg_remove(struct ofono_cdma_netreg *netreg) > +{ > + struct netreg_data *nd =3D ofono_cdma_netreg_get_data(netreg); > + > + ofono_cdma_netreg_set_data(netreg, NULL); > + > + g_at_chat_unref(nd->chat); > + g_free(nd); > +} > + > +static struct ofono_cdma_netreg_driver driver =3D { > + .name =3D "cdmamodem", > + .probe =3D cdma_netreg_probe, > + .remove =3D cdma_netreg_remove, > +}; No need for super extra tabs in between. Cut them to a minimum. > + > +void cdma_netreg_init(void) > +{ > + ofono_cdma_netreg_driver_register(&driver); > +} > + > +void cdma_netreg_exit(void) > +{ > + ofono_cdma_netreg_driver_unregister(&driver); > +} Regards Marcel --===============2619564221995061714==--