Re: [PATCH v2] Add CDMA/EVDO tasks in TODO
Hi Caiwen, On 10/27/2010 08:12 PM, Zhang Caiwen wrote: > --- > TODO | 52 > 1 files changed, 52 insertions(+), 0 deletions(-) > > diff --git a/TODO b/TODO > index 239bab0..e368070 100644 > --- a/TODO > +++ b/TODO > @@ -543,3 +543,55 @@ Miscellaneous > >Priority: Low >Complexity: C4 > + > + > +CDMA/EVDO > += > + > +- Extend modem interface with a 'Mode' property to indicate the modem type. > + > + Priority: High > + Complexity: C1 > + > +- Extend radio setting,add CDMA radio access technology support. The CDMA > modem Can we watch that punctuation is done properly? Space after the comma, preferably two spaces after each period. > + preferred technology can be CDMA, HDR or CDMA/HDR hybrid. > + > + Priority: Medium > + Complexity: C1 > + Owner: Zhang Caiwen > + > +- Add CDMA Network registration support. CDMA device will register to network > + automaticlly. Unlike GSM,generally,it don't support manually register to Can you check the spelling as well? And again, the punctuation. > + network.So its mainly function is to get CDMA network information. The > + information include: registration status, service domain, CDMA 1x signal > + strength, HDR signal strength, MCC, MNC, SID, NID etc. > + > + Priority: Medium > + Complexity: C1 > + Owner: Zhang Caiwen > + > +- Add UIM support. UIM support change pin, enter pin, reset pin, set pin > lock, > + get pin lock setting, get pin remain retry time, and get information that > + stored in UIM, include MCC/MNC, ruimid, IMSI,MDN, PRL version etc. > + > + Priority: High > + Complexity: C4 > + Owner: Zhang Caiwen > + > +- Add CDMA voice call support. CDMA voice call will support > originate/end/hold > + a call,transfer a call, 3WC(three-way calling), conference call and DTMF. > For > + a voice call, can get its status, CLI and ending result(if it is end). > + > + Priority: High > + Complexity: C8 > + Owner: Zhang Zhengguang > + > +- Add CDMA Data connection support. CDMA data connection manager support > + start/end a data connect according to the settings from other module(upper > + layer or ConnMan). > + > + Priority: High > + Complexity: C4 > + Owner: Zhang Caiwen > + > + As a general comment, please try to stick to the wording that the rest of the TODO uses. E.g.: Foo Support. 'Add foo property...' or 'Extend foo to support foo...' The TODO should be accessible to non-telephony geeks. Think of it as a high level design and roadmap document, with the same level of importance. Regards, -Denis ___ ofono mailing list ofono@ofono.org http://lists.ofono.org/listinfo/ofono
Re: [RFC PATCH 2/3] voicecall: emergency call handling added
Hi Andras,
On 10/25/2010 03:03 AM, Andras Domokos wrote:
> From: Andras Domokos
>
>
> Signed-off-by: Andras Domokos
> ---
> include/voicecall.h | 12 +++
> src/voicecall.c | 221 ++
> 2 files changed, 215 insertions(+), 18 deletions(-)
>
> diff --git a/include/voicecall.h b/include/voicecall.h
> index 2356fcf..d530148 100644
> --- a/include/voicecall.h
> +++ b/include/voicecall.h
> @@ -38,6 +38,9 @@ typedef void (*ofono_call_list_cb_t)(const struct
> ofono_error *error,
> const struct ofono_call *call_list,
> void *data);
>
> +typedef void (*ofono_voicecall_emergency_notify_cb_t)(ofono_bool_t state,
> + void *data);
> +
> /* Voice call related functionality, including ATD, ATA, +CHLD, CTFR, CLCC
> * and VTS.
> *
> @@ -116,6 +119,15 @@ void ofono_voicecall_set_data(struct ofono_voicecall
> *vc, void *data);
> void *ofono_voicecall_get_data(struct ofono_voicecall *vc);
> int ofono_voicecall_get_next_callid(struct ofono_voicecall *vc);
>
> +unsigned int __ofono_voicecall_add_emergency_watch(struct ofono_voicecall
> *vc,
> + ofono_voicecall_emergency_notify_cb_t notify,
> + void *data, ofono_destroy_func destroy);
> +void __ofono_voicecall_remove_emergency_watch(struct ofono_voicecall *vc,
> + unsigned int id);
> +ofono_bool_t ofono_voicecall_get_emergency_state(struct ofono_voicecall *vc);
> +void __ofono_voicecall_set_emergency_state(struct ofono_voicecall *vc,
> + int state);
> +
Just some general comments, but this patch seems to be backwards from
the earlier proposal. Namely EmergencyMode is a property on the Modem
interface, not on the VoiceCallManager. See doc/modem-api.txt,
Emergency property.
In general I think that the emergency_watch is unnecessary. Having a
reference counted emergency tracking inside the modem object and a modem
online state watch should be sufficient.
> #ifdef __cplusplus
> }
> #endif
> diff --git a/src/voicecall.c b/src/voicecall.c
> index 7b5fe3b..a619b30 100644
> --- a/src/voicecall.c
> +++ b/src/voicecall.c
> @@ -25,6 +25,7 @@
>
> #include
> #include
> +#include
> #include
> #include
> #include
> @@ -56,6 +57,9 @@ struct ofono_voicecall {
> void *driver_data;
> struct ofono_atom *atom;
> struct dial_request *dial_req;
> + struct ofono_watchlist *emergency_watches;
> + unsigned int emergency_state;
> + unsigned int modem_state_watch;
> };
>
> struct voicecall {
> @@ -85,6 +89,8 @@ static const char *default_en_list_no_sim[] = { "119",
> "118", "999", "110",
>
> static void generic_callback(const struct ofono_error *error, void *data);
> static void multirelease_callback(const struct ofono_error *err, void *data);
> +static const char *voicecall_build_path(struct ofono_voicecall *vc,
> + const struct ofono_call *call);
>
It is generally against the coding style to forward-declare static
functions. If this function is needed, please simply move it higher.
> static gint call_compare_by_id(gconstpointer a, gconstpointer b)
> {
> @@ -121,6 +127,145 @@ static void add_to_en_list(GSList **l, const char
> **list)
> *l = g_slist_prepend(*l, g_strdup(list[i++]));
> }
>
> +static gint number_compare(gconstpointer a, gconstpointer b)
> +{
> + const char *s1 = a, *s2 = b;
> + return strcmp(s1, s2);
> +}
> +
> +static ofono_bool_t emergency_number(struct ofono_voicecall *vc,
> + const char *number)
> +{
> + if (!number)
> + return FALSE;
> +
> + return g_slist_find_custom(vc->en_list,
> + number, number_compare) ? TRUE : FALSE;
> +}
> +
Please add this as a separate patch handling the 'Extend the voicecall
interface with a property indicating whether this call is an emergency
call' task.
> +static void notify_emergency_watches(struct ofono_voicecall *vc,
> + ofono_bool_t state)
> +{
> + struct ofono_watchlist_item *item;
> + GSList *l;
> + ofono_voicecall_emergency_notify_cb_t notify;
> +
> + if (vc->emergency_watches == NULL)
> + return;
> +
> + for (l = vc->emergency_watches->items; l; l = l->next) {
> + item = l->data;
> + notify = item->notify;
> + notify(state, item->notify_data);
> + }
> +}
> +
> +unsigned int __ofono_voicecall_add_emergency_watch(struct ofono_voicecall
> *vc,
> + ofono_voicecall_emergency_notify_cb_t notify,
> + void *data, ofono_destroy_func destroy)
> +{
> + struct ofono_watchlist_item *item;
> +
> + if (vc == NULL || notify == NULL)
> +
Re: [RFC PATCH 1/3] modem: modem state watch added
Hi Andras,
On 10/25/2010 03:03 AM, Andras Domokos wrote:
> From: Andras Domokos
>
> diff --git a/src/ofono.h b/src/ofono.h
> index 6c7f649..b132727 100644
> --- a/src/ofono.h
> +++ b/src/ofono.h
> @@ -177,6 +177,21 @@ unsigned int
> __ofono_modemwatch_add(ofono_modemwatch_cb_t cb, void *user,
> ofono_destroy_func destroy);
> gboolean __ofono_modemwatch_remove(unsigned int id);
>
> +enum ofono_modem_state {
> + MODEM_STATE_POWER_OFF,
> + MODEM_STATE_PRE_SIM,
> + MODEM_STATE_OFFLINE,
> + MODEM_STATE_ONLINE,
Please see doc/coding-style.txt item M11.
> +};
> +
> +typedef void (*ofono_modem_state_notify_func)(enum ofono_modem_state state,
> + void *data);
> +unsigned int __ofono_modem_add_state_watch(struct ofono_modem *modem,
> + ofono_modem_state_notify_func notify,
> + void *data, ofono_destroy_func destroy);
> +void __ofono_modem_remove_state_watch(struct ofono_modem *modem,
> + unsigned int id);
> +
I believe that me and Pekka already agreed not to use MODEM_STATE, but
instead only watch for the online state of the modem.
> #include
>
> gboolean __ofono_call_barring_is_busy(struct ofono_call_barring *cb);
Regards,
-Denis
___
ofono mailing list
ofono@ofono.org
http://lists.ofono.org/listinfo/ofono
Re: [PATCH 6/6] TODO: mark fast dormancy as done
Hi Mika, On 10/26/2010 10:31 AM, Mika Liljeberg wrote: > --- > TODO | 20 > doc/features.txt |8 > 2 files changed, 8 insertions(+), 20 deletions(-) > Applied, thanks. Regards, -Denis ___ ofono mailing list ofono@ofono.org http://lists.ofono.org/listinfo/ofono
Re: [PATCH 5/6] isimodem: add support for FastDormancy property
Hi Mika, On 10/26/2010 10:31 AM, Mika Liljeberg wrote: > --- > drivers/isimodem/radio-settings.c | 91 > - > 1 files changed, 90 insertions(+), 1 deletions(-) > Patch has been applied, thanks. Regards, -Denis ___ ofono mailing list ofono@ofono.org http://lists.ofono.org/listinfo/ofono
Re: [PATCH 4/6] test: add script to control fast dormancy
Hi Mika, On 10/26/2010 10:31 AM, Mika Liljeberg wrote: > --- > Makefile.am|3 ++- > test/set-fast-dormancy | 25 + > 2 files changed, 27 insertions(+), 1 deletions(-) > create mode 100755 test/set-fast-dormancy Patch has been applied, thanks. Regards, -Denis ___ ofono mailing list ofono@ofono.org http://lists.ofono.org/listinfo/ofono
Re: [PATCH 3/6] radio settings: add FastDormancy property
Hi Mika, On 10/26/2010 10:31 AM, Mika Liljeberg wrote: > --- > include/radio-settings.h | 13 + > src/radio-settings.c | 114 +++-- > 2 files changed, 121 insertions(+), 6 deletions(-) > Patch has been applied, thanks. I did make a couple of minor tweaks afterwards. Regards, -Denis ___ ofono mailing list ofono@ofono.org http://lists.ofono.org/listinfo/ofono
Re: [PATCH 1/6] radio settings: fix mode initializion
Hi Mika, On 10/26/2010 10:31 AM, Mika Liljeberg wrote: > --- > include/radio-settings.h |9 + > src/radio-settings.c |2 +- > 2 files changed, 6 insertions(+), 5 deletions(-) I decided to skip this patch. Regards, -Denis ___ ofono mailing list ofono@ofono.org http://lists.ofono.org/listinfo/ofono
[PATCH v2] Add CDMA/EVDO tasks in TODO
--- TODO | 52 1 files changed, 52 insertions(+), 0 deletions(-) diff --git a/TODO b/TODO index 239bab0..e368070 100644 --- a/TODO +++ b/TODO @@ -543,3 +543,55 @@ Miscellaneous Priority: Low Complexity: C4 + + +CDMA/EVDO += + +- Extend modem interface with a 'Mode' property to indicate the modem type. + + Priority: High + Complexity: C1 + +- Extend radio setting,add CDMA radio access technology support. The CDMA modem + preferred technology can be CDMA, HDR or CDMA/HDR hybrid. + + Priority: Medium + Complexity: C1 + Owner: Zhang Caiwen + +- Add CDMA Network registration support. CDMA device will register to network + automaticlly. Unlike GSM,generally,it don't support manually register to + network.So its mainly function is to get CDMA network information. The + information include: registration status, service domain, CDMA 1x signal + strength, HDR signal strength, MCC, MNC, SID, NID etc. + + Priority: Medium + Complexity: C1 + Owner: Zhang Caiwen + +- Add UIM support. UIM support change pin, enter pin, reset pin, set pin lock, + get pin lock setting, get pin remain retry time, and get information that + stored in UIM, include MCC/MNC, ruimid, IMSI,MDN, PRL version etc. + + Priority: High + Complexity: C4 + Owner: Zhang Caiwen + +- Add CDMA voice call support. CDMA voice call will support originate/end/hold + a call,transfer a call, 3WC(three-way calling), conference call and DTMF. For + a voice call, can get its status, CLI and ending result(if it is end). + + Priority: High + Complexity: C8 + Owner: Zhang Zhengguang + +- Add CDMA Data connection support. CDMA data connection manager support + start/end a data connect according to the settings from other module(upper + layer or ConnMan). + + Priority: High + Complexity: C4 + Owner: Zhang Caiwen + + -- 1.7.0.4 ___ ofono mailing list ofono@ofono.org http://lists.ofono.org/listinfo/ofono
Re: [PATCH] Fix string array memory leaks during plugin loading
Hi Johan, > src/plugin.c |3 +++ > 1 files changed, 3 insertions(+), 0 deletions(-) patch has been applied. Regards Marcel ___ ofono mailing list ofono@ofono.org http://lists.ofono.org/listinfo/ofono
[PATCH] Fix string array memory leaks during plugin loading
From: Johan Hedberg --- src/plugin.c |3 +++ 1 files changed, 3 insertions(+), 0 deletions(-) diff --git a/src/plugin.c b/src/plugin.c index 68fe318..7c0652b 100644 --- a/src/plugin.c +++ b/src/plugin.c @@ -176,6 +176,9 @@ int __ofono_plugin_init(const char *pattern, const char *exclude) plugin->active = TRUE; } + g_strfreev(patterns); + g_strfreev(excludes); + return 0; } -- 1.7.1 ___ ofono mailing list ofono@ofono.org http://lists.ofono.org/listinfo/ofono
Re: [PATCH] Add CDMA/EVDO tasks in TODO
Hi Caiwen, On 10/27/2010 06:16 AM, Zhang Caiwen wrote: > --- > TODO | 57 + > 1 files changed, 57 insertions(+), 0 deletions(-) > > diff --git a/TODO b/TODO > index 239bab0..efb2bff 100644 > --- a/TODO > +++ b/TODO > @@ -543,3 +543,60 @@ Miscellaneous > >Priority: Low >Complexity: C4 > + > + > +CDMA/EVDO > += > + > +- Extend modem interface with a 'Mode' property to indicate the modem type. > + > + Priority: High > + Complexity: C1 > + > +- Extend radio setting,add CDMA radio access technology support. The CDMA > modem > + preferred technology can be CDMA, HDR or CDMA/HDR hybrid. > + > + Priority: Medium > + Complexity: C1 > + Owner: Zhang Caiwen > + > +- Add CDMA Network registration support. CDMA device will register to network > + automaticlly. Unlike GSM,it don't support manually register to network.So > its > + mainly function is to get CDMA network information. The information > include: > + registration status, service domain, CDMA 1x signal strength, HDR signal > + strength, MCC, MNC, SID, NID etc. > + > + Priority: Medium > + Complexity: C1 > + Owner: Zhang Caiwen > + > +- Add UIM support. UIM support change pin, enter pin, reset pin, set pin > lock, > + get pin lock setting, get pin remain retry time, and get information that > + stored in UIM, include MCC/MNC, ruimid, IMSI,MDN, PRL version etc. > + > + Priority: High > + Complexity: C4 > + Owner: Zhang Caiwen > + > +- Add CDMA voice call support. CDMA voice call will support > originate/end/hold > + a call,transfer a call, 3WC(three-way calling), conference call and DTMF. > For > + a voice call, can get its status, CLI and ending result(if it is end). > + > + Priority: High > + Complexity: C6 Please don't invent new complexity types. If it is higher than a C4, mark it as a C8. > + Owner: Zhang Zhengguang > + > +- Add CDMA Data connection support. CDMA data connection manager support > + start/end a data connect according to the settings from other module(upper > + layer or ConnMan). > + > + Priority: High > + Complexity: C4 > + Owner: Zhang Caiwen > + > +- Add SIM/UIM data access support. Add a card atom, it will support > read/update/ > + delete/add conatact add SMS in SIM/UIM card. > + You do realize oFono doesn't really work this way. Certainly update/delete/add contacts is not going to be accepted as a task. And definitely a big no for SMS to the SIM/UIM. > + Priority: High > + Complexity: C6 Same comment as above. > + Regards, -Denis ___ ofono mailing list ofono@ofono.org http://lists.ofono.org/listinfo/ofono
Re: [PATCH] Document Proxy-CSCF setting.
Hi Pekka, > Proxy-CSCF is a SIP proxy for IMS connections. > --- > doc/connman-api.txt |5 + > 1 files changed, 5 insertions(+), 0 deletions(-) I am not going to apply this patch right now. Things with IMS are still not really certain to me. And until there is an implementation that goes along with this, it is a bit early. Regards Marcel ___ ofono mailing list ofono@ofono.org http://lists.ofono.org/listinfo/ofono
Re: [PATCH] Document ims connection type
Hi Pekka, > doc/connman-api.txt |1 + > 1 files changed, 1 insertions(+), 0 deletions(-) patch has been applied. Regards Marcel ___ ofono mailing list ofono@ofono.org http://lists.ofono.org/listinfo/ofono
Re: [PATCH] gprs-context: add IMS connection type
Hi Pekka, > include/gprs-context.h |1 + > src/gprs.c |6 ++ > 2 files changed, 7 insertions(+), 0 deletions(-) patch has been applied. Regards Marcel ___ ofono mailing list ofono@ofono.org http://lists.ofono.org/listinfo/ofono
[PATCH] Document Proxy-CSCF setting.
From: Pekka Pessi
Proxy-CSCF is a SIP proxy for IMS connections.
---
doc/connman-api.txt |5 +
1 files changed, 5 insertions(+), 0 deletions(-)
diff --git a/doc/connman-api.txt b/doc/connman-api.txt
index dc94a89..872589e 100644
--- a/doc/connman-api.txt
+++ b/doc/connman-api.txt
@@ -212,6 +212,11 @@ Properties boolean Active [readwrite]
Holds the gateway IP for this connection.
+ array{string} ProxyCSCF [readonly, optional]
+
+ Holds the list of IMS P-CSCF servers for
+ this connection.
+
string MessageProxy [readwrite, MMS only]
Holds the MMS Proxy setting.
--
1.7.1
___
ofono mailing list
ofono@ofono.org
http://lists.ofono.org/listinfo/ofono
[PATCH] Document ims connection type
From: Pekka Pessi --- doc/connman-api.txt |1 + 1 files changed, 1 insertions(+), 0 deletions(-) diff --git a/doc/connman-api.txt b/doc/connman-api.txt index 9fe620f..dc94a89 100644 --- a/doc/connman-api.txt +++ b/doc/connman-api.txt @@ -155,6 +155,7 @@ Properties boolean Active [readwrite] "internet" - General internet connectivity "mms" - Used by MMS related services "wap" - Used by WAP related services + "ims" - Used by IMS related services string Username [readwrite] -- 1.7.1 ___ ofono mailing list ofono@ofono.org http://lists.ofono.org/listinfo/ofono
[PATCH] gprs-context: add IMS connection type
From: Pekka Pessi
---
include/gprs-context.h |1 +
src/gprs.c |6 ++
2 files changed, 7 insertions(+), 0 deletions(-)
diff --git a/include/gprs-context.h b/include/gprs-context.h
index 19abf33..f796e2d 100644
--- a/include/gprs-context.h
+++ b/include/gprs-context.h
@@ -44,6 +44,7 @@ enum ofono_gprs_context_type {
OFONO_GPRS_CONTEXT_TYPE_INTERNET,
OFONO_GPRS_CONTEXT_TYPE_MMS,
OFONO_GPRS_CONTEXT_TYPE_WAP,
+ OFONO_GPRS_CONTEXT_TYPE_IMS,
};
struct ofono_gprs_primary_context {
diff --git a/src/gprs.c b/src/gprs.c
index 1c8ab50..222a52d 100644
--- a/src/gprs.c
+++ b/src/gprs.c
@@ -128,6 +128,8 @@ static const char *gprs_context_type_to_default_name(enum
ofono_gprs_context_typ
return "MMS";
case OFONO_GPRS_CONTEXT_TYPE_WAP:
return "WAP";
+ case OFONO_GPRS_CONTEXT_TYPE_IMS:
+ return "IMS";
}
return NULL;
@@ -144,6 +146,8 @@ static const char *gprs_context_type_to_string(enum
ofono_gprs_context_type type
return "mms";
case OFONO_GPRS_CONTEXT_TYPE_WAP:
return "wap";
+ case OFONO_GPRS_CONTEXT_TYPE_IMS:
+ return "ims";
}
return NULL;
@@ -157,6 +161,8 @@ static enum ofono_gprs_context_type
gprs_context_string_to_type(const char *str)
return OFONO_GPRS_CONTEXT_TYPE_WAP;
else if (g_str_equal(str, "mms"))
return OFONO_GPRS_CONTEXT_TYPE_MMS;
+ else if (g_str_equal(str, "ims"))
+ return OFONO_GPRS_CONTEXT_TYPE_IMS;
return OFONO_GPRS_CONTEXT_TYPE_INVALID;
}
--
1.7.1
___
ofono mailing list
ofono@ofono.org
http://lists.ofono.org/listinfo/ofono
[PATCH] Add CDMA/EVDO tasks in TODO
--- TODO | 57 + 1 files changed, 57 insertions(+), 0 deletions(-) diff --git a/TODO b/TODO index 239bab0..efb2bff 100644 --- a/TODO +++ b/TODO @@ -543,3 +543,60 @@ Miscellaneous Priority: Low Complexity: C4 + + +CDMA/EVDO += + +- Extend modem interface with a 'Mode' property to indicate the modem type. + + Priority: High + Complexity: C1 + +- Extend radio setting,add CDMA radio access technology support. The CDMA modem + preferred technology can be CDMA, HDR or CDMA/HDR hybrid. + + Priority: Medium + Complexity: C1 + Owner: Zhang Caiwen + +- Add CDMA Network registration support. CDMA device will register to network + automaticlly. Unlike GSM,it don't support manually register to network.So its + mainly function is to get CDMA network information. The information include: + registration status, service domain, CDMA 1x signal strength, HDR signal + strength, MCC, MNC, SID, NID etc. + + Priority: Medium + Complexity: C1 + Owner: Zhang Caiwen + +- Add UIM support. UIM support change pin, enter pin, reset pin, set pin lock, + get pin lock setting, get pin remain retry time, and get information that + stored in UIM, include MCC/MNC, ruimid, IMSI,MDN, PRL version etc. + + Priority: High + Complexity: C4 + Owner: Zhang Caiwen + +- Add CDMA voice call support. CDMA voice call will support originate/end/hold + a call,transfer a call, 3WC(three-way calling), conference call and DTMF. For + a voice call, can get its status, CLI and ending result(if it is end). + + Priority: High + Complexity: C6 + Owner: Zhang Zhengguang + +- Add CDMA Data connection support. CDMA data connection manager support + start/end a data connect according to the settings from other module(upper + layer or ConnMan). + + Priority: High + Complexity: C4 + Owner: Zhang Caiwen + +- Add SIM/UIM data access support. Add a card atom, it will support read/update/ + delete/add conatact add SMS in SIM/UIM card. + + Priority: High + Complexity: C6 + -- 1.7.0.4 ___ ofono mailing list ofono@ofono.org http://lists.ofono.org/listinfo/ofono
RE: [RFC] AGPS Support
Hi My two cents... > -Original Message- > > > I think the oFono interface should not use binary data, but rather be > aligned with 27.007 > > and present decoded data as DBUS typed signals and methods with the > same information > > content as specified in the XML data for AT+CPOS, AT+CPOSR. > > This was proposed several times. The main objections against the XML > file format was the difficulty of parsing such a beast and support of > it > by modem vendors. > > Personally (and this is based on rather limited experience with this > area) I'm not against the idea; it fits with oFono's overall strategy > of > following 27.007 whenever possible... AFAIK, it does not exist a free standard API for satellite measurements and GPS assistance data. Moreover, while Galileo and Compass GNSS are not yet deployed, GLONASS support will be soon a need. So, the idea is to not interpret positioning data or request and to pass them transparently from the modem to the positioning framework and vis versa. On positioning framework side, XML containers encoding/decoding does not seem to be yet widely used, while the support of RRC and RRLP framing is common, thanks to the support of SUPL. Using XML and 27.007 format would be great as long as long as ofono is not obliged to interpret the XML containers. BR, Fred - Intel Corporation SAS (French simplified joint stock company) Registered headquarters: "Les Montalets"- 2, rue de Paris, 92196 Meudon Cedex, France Registration Number: 302 456 199 R.C.S. NANTERRE Capital: 4,572,000 Euros This e-mail and any attachments may contain confidential material for the sole use of the intended recipient(s). Any review or distribution by others is strictly prohibited. If you are not the intended recipient, please contact the sender and delete all copies. ___ ofono mailing list ofono@ofono.org http://lists.ofono.org/listinfo/ofono
RE: [PATCH 2/6] stemodem: add default case
Hi Mika,
> > > That must be a new policy then, considering that stemodem
> > is the only one that failed compilation. Feel free to fix
> > this one. The first two patches in the set are unrelated to
> > fast dormancy anyway.
> >
> > that is the whole point here. You modified the enum and the
> > compilation
> > should fail unless you add a statement for that new item.
> >
> > Let me make this clear, I do want the compilation to fail here and the
> > STE driver is doing the right thing.
>
> I understand that. As I said, feel free to fix. Fast dormancy patches 3-6
> should apply just fine without the first two courtesy patches.
I am waiting for Denis review since he was looking at them initially.
Once these are applied, we can take care of the fallout.
> > There might be other cases where this is not consistent. I
> > would prefer
> > if that never happens, but somethings things slip through even close
> > code review. If you know other cases, please let me know and
> > we fix them
> > as well here.
>
> All the other drivers implementing radio settings. Here's a couple of
> examples:
>
> ac63fd95 (Marcel Holtmann 2010-09-23 23:27:08 +0900 134)switch (mode)
> {
> ac63fd95 (Marcel Holtmann 2010-09-23 23:27:08 +0900 135)case
> OFONO_RADIO_ACCESS_MODE_ANY:
> ac63fd95 (Marcel Holtmann 2010-09-23 23:27:08 +0900 136)value
> = 1;
> ac63fd95 (Marcel Holtmann 2010-09-23 23:27:08 +0900 137)break;
> ac63fd95 (Marcel Holtmann 2010-09-23 23:27:08 +0900 138)case
> OFONO_RADIO_ACCESS_MODE_GSM:
> ac63fd95 (Marcel Holtmann 2010-09-23 23:27:08 +0900 139)value
> = 0;
> ac63fd95 (Marcel Holtmann 2010-09-23 23:27:08 +0900 140)break;
> ac63fd95 (Marcel Holtmann 2010-09-23 23:27:08 +0900 141)case
> OFONO_RADIO_ACCESS_MODE_UMTS:
> ac63fd95 (Marcel Holtmann 2010-09-23 23:27:08 +0900 142)value
> = 2;
> ac63fd95 (Marcel Holtmann 2010-09-23 23:27:08 +0900 143)break;
> ac63fd95 (Marcel Holtmann 2010-09-23 23:27:08 +0900 144)default:
> ac63fd95 (Marcel Holtmann 2010-09-23 23:27:08 +0900 145)
> CALLBACK_WITH_FAILURE(cb, data);
> ac63fd95 (Marcel Holtmann 2010-09-23 23:27:08 +0900 146)
> g_free(cbd);
> ac63fd95 (Marcel Holtmann 2010-09-23 23:27:08 +0900 147)
> return;
> ac63fd95 (Marcel Holtmann 2010-09-23 23:27:08 +0900 148)}
>
> 30b054d0 (Marcel Holtmann 2010-06-06 15:18:57 -0700 133)switch (mode)
> {
> 30b054d0 (Marcel Holtmann 2010-06-06 15:18:57 -0700 134)case
> OFONO_RADIO_ACCESS_MODE_ANY:
> 30b054d0 (Marcel Holtmann 2010-06-06 15:18:57 -0700 135)value
> = 5;
> 30b054d0 (Marcel Holtmann 2010-06-06 15:18:57 -0700 136)break;
> 30b054d0 (Marcel Holtmann 2010-06-06 15:18:57 -0700 137)case
> OFONO_RADIO_ACCESS_MODE_GSM:
> 30b054d0 (Marcel Holtmann 2010-06-06 15:18:57 -0700 138)value
> = 0;
> 30b054d0 (Marcel Holtmann 2010-06-06 15:18:57 -0700 139)break;
> 30b054d0 (Marcel Holtmann 2010-06-06 15:18:57 -0700 140)case
> OFONO_RADIO_ACCESS_MODE_UMTS:
> 30b054d0 (Marcel Holtmann 2010-06-06 15:18:57 -0700 141)value
> = 1;
> 30b054d0 (Marcel Holtmann 2010-06-06 15:18:57 -0700 142)break;
> 30b054d0 (Marcel Holtmann 2010-06-06 15:18:57 -0700 143)default:
> 30b054d0 (Marcel Holtmann 2010-06-06 15:18:57 -0700 144)
> CALLBACK_WITH_FAILURE(cb, data);
> 30b054d0 (Marcel Holtmann 2010-06-06 15:18:57 -0700 145)
> g_free(cbd);
> 30b054d0 (Marcel Holtmann 2010-06-06 15:18:57 -0700 146)
> return;
> 30b054d0 (Marcel Holtmann 2010-06-06 15:18:57 -0700 147)}
>
> Sorry, couldn't resist. ;-)
I have no problem with that being pointing out. I actually encourage
people pointing this out. This is clearly myself being stupid. And good
thing is that this is open source, so I fixed it.
With a code base of 100k lines of code and more, such things happen. And
even with Denis and myself cross-reviewing each other this will still
happen. Sometimes things just slip through the cracks. Part of
development life. So once noticed this will be dealt with.
Regards
Marcel
___
ofono mailing list
ofono@ofono.org
http://lists.ofono.org/listinfo/ofono
RE: [PATCH 2/6] stemodem: add default case
Hi Marcel,
> > That must be a new policy then, considering that stemodem
> is the only one that failed compilation. Feel free to fix
> this one. The first two patches in the set are unrelated to
> fast dormancy anyway.
>
> that is the whole point here. You modified the enum and the
> compilation
> should fail unless you add a statement for that new item.
>
> Let me make this clear, I do want the compilation to fail here and the
> STE driver is doing the right thing.
I understand that. As I said, feel free to fix. Fast dormancy patches 3-6
should apply just fine without the first two courtesy patches.
> There might be other cases where this is not consistent. I
> would prefer
> if that never happens, but somethings things slip through even close
> code review. If you know other cases, please let me know and
> we fix them
> as well here.
All the other drivers implementing radio settings. Here's a couple of examples:
ac63fd95 (Marcel Holtmann 2010-09-23 23:27:08 +0900 134)switch (mode) {
ac63fd95 (Marcel Holtmann 2010-09-23 23:27:08 +0900 135)case
OFONO_RADIO_ACCESS_MODE_ANY:
ac63fd95 (Marcel Holtmann 2010-09-23 23:27:08 +0900 136)value =
1;
ac63fd95 (Marcel Holtmann 2010-09-23 23:27:08 +0900 137)break;
ac63fd95 (Marcel Holtmann 2010-09-23 23:27:08 +0900 138)case
OFONO_RADIO_ACCESS_MODE_GSM:
ac63fd95 (Marcel Holtmann 2010-09-23 23:27:08 +0900 139)value =
0;
ac63fd95 (Marcel Holtmann 2010-09-23 23:27:08 +0900 140)break;
ac63fd95 (Marcel Holtmann 2010-09-23 23:27:08 +0900 141)case
OFONO_RADIO_ACCESS_MODE_UMTS:
ac63fd95 (Marcel Holtmann 2010-09-23 23:27:08 +0900 142)value =
2;
ac63fd95 (Marcel Holtmann 2010-09-23 23:27:08 +0900 143)break;
ac63fd95 (Marcel Holtmann 2010-09-23 23:27:08 +0900 144)default:
ac63fd95 (Marcel Holtmann 2010-09-23 23:27:08 +0900 145)
CALLBACK_WITH_FAILURE(cb, data);
ac63fd95 (Marcel Holtmann 2010-09-23 23:27:08 +0900 146)
g_free(cbd);
ac63fd95 (Marcel Holtmann 2010-09-23 23:27:08 +0900 147)return;
ac63fd95 (Marcel Holtmann 2010-09-23 23:27:08 +0900 148)}
30b054d0 (Marcel Holtmann 2010-06-06 15:18:57 -0700 133)switch (mode) {
30b054d0 (Marcel Holtmann 2010-06-06 15:18:57 -0700 134)case
OFONO_RADIO_ACCESS_MODE_ANY:
30b054d0 (Marcel Holtmann 2010-06-06 15:18:57 -0700 135)value =
5;
30b054d0 (Marcel Holtmann 2010-06-06 15:18:57 -0700 136)break;
30b054d0 (Marcel Holtmann 2010-06-06 15:18:57 -0700 137)case
OFONO_RADIO_ACCESS_MODE_GSM:
30b054d0 (Marcel Holtmann 2010-06-06 15:18:57 -0700 138)value =
0;
30b054d0 (Marcel Holtmann 2010-06-06 15:18:57 -0700 139)break;
30b054d0 (Marcel Holtmann 2010-06-06 15:18:57 -0700 140)case
OFONO_RADIO_ACCESS_MODE_UMTS:
30b054d0 (Marcel Holtmann 2010-06-06 15:18:57 -0700 141)value =
1;
30b054d0 (Marcel Holtmann 2010-06-06 15:18:57 -0700 142)break;
30b054d0 (Marcel Holtmann 2010-06-06 15:18:57 -0700 143)default:
30b054d0 (Marcel Holtmann 2010-06-06 15:18:57 -0700 144)
CALLBACK_WITH_FAILURE(cb, data);
30b054d0 (Marcel Holtmann 2010-06-06 15:18:57 -0700 145)
g_free(cbd);
30b054d0 (Marcel Holtmann 2010-06-06 15:18:57 -0700 146)return;
30b054d0 (Marcel Holtmann 2010-06-06 15:18:57 -0700 147)}
Sorry, couldn't resist. ;-)
Br,
MikaL
___
ofono mailing list
ofono@ofono.org
http://lists.ofono.org/listinfo/ofono
