[PATCH 0/5] Implement CPHS CSP support

[PATCH 0/5] Implement CPHS CSP support

[Aki Niemi]

[-- Attachment #1: Type: text/plain, Size: 1167 bytes --]

Hi All,

This patch set implements support for the PLMN mode bit found in the
CPHS EFcsp file. Based on the PLMN mode setting, the netreg atom
enables or disables manual network registration.

There is a new Mode property value of 'forced-auto' for indicating the
case when manual selection is disabled. Any attempt to manually
Register() will also fail with an error.

In addition, we add a short description of the CPHS spec in
doc/standards.txt. Unfortunately that spec is not publically
available, although people may have luck finding an unofficial version
online with the help of a search engine. ;)

Aki Niemi (5):
  simutil: Add EFcsp file and service group IDs
  netreg: Add CPHS CSP PLMN mode implementation
  doc: Add documentation for 'forced-mode'
  doc: Add short mention of CPHS under standards
  TODO: Remove completed CPHS CSP task

 TODO                |   13 --------
 doc/network-api.txt |   24 ++++++++------
 doc/standards.txt   |    9 +++++
 src/network.c       |   85 ++++++++++++++++++++++++++++++++++++++++++++++++++-
 src/simutil.h       |   16 +++++++++
 5 files changed, 123 insertions(+), 24 deletions(-)

[PATCH 1/5] simutil: Add EFcsp file and service group IDs

[Aki Niemi]

[-- Attachment #1: Type: text/plain, Size: 1273 bytes --]

---
 src/simutil.h |   16 ++++++++++++++++
 1 files changed, 16 insertions(+), 0 deletions(-)

diff --git a/src/simutil.h b/src/simutil.h
index 7e386fa..61440a5 100644
--- a/src/simutil.h
+++ b/src/simutil.h
@@ -26,6 +26,7 @@ enum sim_fileid {
 	SIM_EFLI_FILEID =			0x6F05,
 	SIM_EF_CPHS_MWIS_FILEID =		0x6F11,
 	SIM_EF_CPHS_CFF_FILEID =		0x6F13,
+	SIM_EF_CPHS_CSP_FILEID =		0x6F15,
 	SIM_EF_CPHS_INFORMATION_FILEID =	0x6F16,
 	SIM_EF_CPHS_MBDN_FILEID =		0x6F17,
 	SIM_EFUST_FILEID =			0x6F38,
@@ -223,6 +224,21 @@ enum sim_sst_service {
 	SIM_SST_SERVICE_PROVIDER_DISPLAY_INFO =		55
 };
 
+/* CPHS 4.2, Section B4.7 CSP Service Group Codes */
+enum sim_csp_entry {
+	SIM_CSP_ENTRY_CALL_OFFERING =		0x01,
+	SIM_CSP_ENTRY_CALL_RESTRICTION =	0x02,
+	SIM_CSP_ENTRY_OTHER_SUPP_SERVICES =	0x03,
+	SIM_CSP_ENTRY_CALL_COMPLETION =		0x04,
+	SIM_CSP_ENTRY_TELESERVICES =		0x05,
+	SIM_CSP_ENTRY_CPHS_TELESERVICES =	0x06,
+	SIM_CSP_ENTRY_CPHS_FEATURES =		0x07,
+	SIM_CSP_ENTRY_NUMBER_IDENTIFICATION =	0x08,
+	SIM_CSP_ENTRY_PHASE_2GPLUS_SERVICES =	0x09,
+	SIM_CSP_ENTRY_VALUE_ADDED_SERVICES =	0xC0,
+	SIM_CSP_ENTRY_INFORMATION_NUMBERS =	0xD5,
+};
+
 #define SIM_EFSPN_DC_HOME_PLMN_BIT 0x1
 #define SIM_EFSPN_DC_ROAMING_SPN_BIT 0x2
 
-- 
1.7.1

[PATCH 2/5] netreg: Add CPHS CSP PLMN mode implementation

[Aki Niemi]

[-- Attachment #1: Type: text/plain, Size: 4415 bytes --]

---
 src/network.c |   85 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
 1 files changed, 84 insertions(+), 1 deletions(-)

diff --git a/src/network.c b/src/network.c
index ae0f334..927f7ce 100644
--- a/src/network.c
+++ b/src/network.c
@@ -66,6 +66,7 @@ struct ofono_netreg {
 	int cellid;
 	int technology;
 	int mode;
+	gboolean forced_auto;
 	char *base_station;
 	struct network_operator_data *current_operator;
 	GSList *operator_list;
@@ -174,6 +175,36 @@ static void set_registration_mode(struct ofono_netreg *netreg, int mode)
 					"Mode", DBUS_TYPE_STRING, &strmode);
 }
 
+static void set_registration_forced_auto(struct ofono_netreg *netreg, gboolean value)
+{
+	DBusConnection *conn;
+	const char *strmode;
+	const char *path;
+
+	if (netreg->forced_auto == value)
+		return;
+
+	netreg->forced_auto = value;
+
+	if (netreg->settings) {
+		g_key_file_set_boolean(netreg->settings, SETTINGS_GROUP,
+					"ForcedAuto", netreg->forced_auto);
+		storage_sync(netreg->imsi, SETTINGS_STORE, netreg->settings);
+	}
+
+	if (value)
+		strmode = "forced-auto";
+	else
+		strmode = registration_mode_to_string(netreg->mode);
+
+	conn = ofono_dbus_get_connection();
+	path = __ofono_atom_get_path(netreg->atom);
+
+	ofono_dbus_signal_property_changed(conn, path,
+					OFONO_NETWORK_REGISTRATION_INTERFACE,
+					"Mode", DBUS_TYPE_STRING, &strmode);
+}
+
 static void registration_status_callback(const struct ofono_error *error,
 					int status, int lac, int ci, int tech,
 					void *data)
@@ -588,6 +619,9 @@ static DBusMessage *network_operator_register(DBusConnection *conn,
 	struct network_operator_data *opd = data;
 	struct ofono_netreg *netreg = opd->netreg;
 
+	if (netreg->forced_auto)
+		return __ofono_error_access_denied(msg);
+
 	if (netreg->pending)
 		return __ofono_error_busy(msg);
 
@@ -757,7 +791,12 @@ static DBusMessage *network_get_properties(DBusConnection *conn,
 
 	const char *status = registration_status_to_string(netreg->status);
 	const char *operator;
-	const char *mode = registration_mode_to_string(netreg->mode);
+	const char *mode;
+
+	if (netreg->forced_auto)
+		mode = "forced-auto";
+	else
+		mode = registration_mode_to_string(netreg->mode);
 
 	reply = dbus_message_new_method_return(msg);
 	if (reply == NULL)
@@ -1598,6 +1637,43 @@ static void sim_spn_read_cb(int ok, int length, int record,
 	}
 }
 
+static void sim_csp_read_cb(int ok, int length, int record,
+				const unsigned char *data,
+				int record_length, void *user_data)
+{
+	struct ofono_netreg *netreg = user_data;
+	int i;
+
+	if (!ok)
+		return;
+
+	if (length < 18 || record_length < 18 || length < record_length)
+		return;
+
+	/*
+	 * Acording to CPHS 4.2, EFcsp is an array of two-byte service
+	 * entries, each consisting of a one byte service group
+	 * identifier followed by 8 bits; each bit is indicating
+	 * availability of a specific service or feature.
+	 *
+	 * The PLMN mode bit, if present, indicates whether manual
+	 * operator selection should be disabled or enabled. When
+	 * unset, the device is forced to automatic mode; when set,
+	 * manual selection is to be enabled. The latter is also the
+	 * default.
+	 */
+	for (i = 0; i < record_length / 2; i++) {
+		gboolean forced_auto;
+
+		if (data[i * 2] != SIM_CSP_ENTRY_VALUE_ADDED_SERVICES)
+			continue;
+
+		forced_auto = (data[i * 2 + 1] & 0x80) == 0;
+
+		set_registration_forced_auto(netreg, forced_auto);
+	}
+}
+
 int ofono_netreg_get_location(struct ofono_netreg *netreg)
 {
 	if (netreg == NULL)
@@ -1803,6 +1879,10 @@ static void netreg_load_settings(struct ofono_netreg *netreg)
 
 	g_key_file_set_integer(netreg->settings, SETTINGS_GROUP,
 				"Mode", netreg->mode);
+
+	netreg->forced_auto = g_key_file_get_boolean(netreg->settings,
+							SETTINGS_GROUP,
+							"ForcedAuto", NULL);
 }
 
 void ofono_netreg_register(struct ofono_netreg *netreg)
@@ -1845,6 +1925,9 @@ void ofono_netreg_register(struct ofono_netreg *netreg)
 		ofono_sim_read(netreg->sim, SIM_EFSPN_FILEID,
 				OFONO_SIM_FILE_STRUCTURE_TRANSPARENT,
 				sim_spn_read_cb, netreg);
+		ofono_sim_read(netreg->sim, SIM_EF_CPHS_CSP_FILEID,
+				OFONO_SIM_FILE_STRUCTURE_TRANSPARENT,
+				sim_csp_read_cb, netreg);
 	}
 
 	__ofono_atom_register(netreg->atom, netreg_unregister);
-- 
1.7.1

Re: [PATCH 2/5] netreg: Add CPHS CSP PLMN mode implementation

[Marcel Holtmann]

[-- Attachment #1: Type: text/plain, Size: 366 bytes --]

Hi Aki,

<snip>

> +	if (netreg->settings) {
> +		g_key_file_set_boolean(netreg->settings, SETTINGS_GROUP,
> +					"ForcedAuto", netreg->forced_auto);
> +		storage_sync(netreg->imsi, SETTINGS_STORE, netreg->settings);
> +	}

why are we bothering to store this value? Would it not be better to
re-read when bringing up the modem?

Regards

Marcel

Re: [PATCH 2/5] netreg: Add CPHS CSP PLMN mode implementation

[Aki Niemi]

[-- Attachment #1: Type: text/plain, Size: 857 bytes --]

Hi Marcel,

2011/1/18 Marcel Holtmann <marcel@holtmann.org>:
>> +     if (netreg->settings) {
>> +             g_key_file_set_boolean(netreg->settings, SETTINGS_GROUP,
>> +                                     "ForcedAuto", netreg->forced_auto);
>> +             storage_sync(netreg->imsi, SETTINGS_STORE, netreg->settings);
>> +     }
>
> why are we bothering to store this value? Would it not be better to
> re-read when bringing up the modem?

We read the value always, yes, but storing the last known value rids
us of a superfluous PropertyChanged signal in the case when last value
of forced_auto was true, and after reading is still true. This is
because forced_auto is by default set to false.

I went back and forth with this myself as well, so perhaps it's not
worth the trouble.

Cheers,
Aki

Re: [PATCH 2/5] netreg: Add CPHS CSP PLMN mode implementation

[Denis Kenzior]

[-- Attachment #1: Type: text/plain, Size: 1027 bytes --]

Hi Aki,

On 01/18/2011 12:44 PM, Aki Niemi wrote:
> Hi Marcel,
> 
> 2011/1/18 Marcel Holtmann <marcel@holtmann.org>:
>>> +     if (netreg->settings) {
>>> +             g_key_file_set_boolean(netreg->settings, SETTINGS_GROUP,
>>> +                                     "ForcedAuto", netreg->forced_auto);
>>> +             storage_sync(netreg->imsi, SETTINGS_STORE, netreg->settings);
>>> +     }
>>
>> why are we bothering to store this value? Would it not be better to
>> re-read when bringing up the modem?
> 
> We read the value always, yes, but storing the last known value rids
> us of a superfluous PropertyChanged signal in the case when last value
> of forced_auto was true, and after reading is still true. This is
> because forced_auto is by default set to false.

Do you mean across atom instantiations here?  If so there are plenty of
other attributes that get emitted in a similar fashion, so my vote is to
just emit the signal.  Or are you worried about something else?

Regards,
-Denis

Re: [PATCH 2/5] netreg: Add CPHS CSP PLMN mode implementation

[Aki Niemi]

[-- Attachment #1: Type: text/plain, Size: 622 bytes --]

Hi Denis,

2011/1/18 Denis Kenzior <denkenz@gmail.com>:
>> We read the value always, yes, but storing the last known value rids
>> us of a superfluous PropertyChanged signal in the case when last value
>> of forced_auto was true, and after reading is still true. This is
>> because forced_auto is by default set to false.
>
> Do you mean across atom instantiations here?  If so there are plenty of
> other attributes that get emitted in a similar fashion, so my vote is to
> just emit the signal.  Or are you worried about something else?

Nope, that was basically it. I'll remove the code.

Cheers,
Aki

[PATCH 3/5] doc: Add documentation for 'forced-mode'

[Aki Niemi]

[-- Attachment #1: Type: text/plain, Size: 1708 bytes --]

---
 doc/network-api.txt |   24 ++++++++++++++----------
 1 files changed, 14 insertions(+), 10 deletions(-)

diff --git a/doc/network-api.txt b/doc/network-api.txt
index c252988..c46e81b 100644
--- a/doc/network-api.txt
+++ b/doc/network-api.txt
@@ -75,21 +75,25 @@ Properties	string Mode [readonly]
 			method of an operator is called.
 
 			The possible values are:
-				"auto"     Network registration is performed
-				           automatically.
-				"manual"   Network operator is selected
-				           manually. If the operator is
-				           currently not selected, registration
-				           is not attempted
-				"off"      Unregisters from currently selected
-				           network, if any, and does not
-				           attempt to register again.
+				"auto"         Network registration is performed
+				               automatically.
+				"forced-auto"  Network registration is performed
+					       automatically while manual
+				               selection is disabled.
+				"manual"       Network operator is selected
+				               manually. If the operator is
+				               currently not selected,
+				               registration is not attempted.
+				"off"          Unregisters from currently
+				               selected network, if any, and
+				               does not attempt to register
+				               again.
 
 		string Status [readonly]
 
 			The current registration status of a modem.
 
-			The possible values are: 
+			The possible values are:
 				"unregistered"  Not registered to any network
 				"registered"    Registered to home network
 				"searching"     Not registered, but searching
-- 
1.7.1

Re: [PATCH 3/5] doc: Add documentation for 'forced-mode'

[Marcel Holtmann]

[-- Attachment #1: Type: text/plain, Size: 471 bytes --]

Hi Aki,

<snip>

> +				"auto"         Network registration is performed
> +				               automatically.
> +				"forced-auto"  Network registration is performed
> +					       automatically while manual
> +				               selection is disabled.

we need to discuss this a little bit. I am not sure using a mode of
forced auto is the best choice here.

Maybe an extra property like "ManualAllowed" would be better. Thoughts?

Regards

Marcel

Re: [PATCH 3/5] doc: Add documentation for 'forced-mode'

[Aki Niemi]

[-- Attachment #1: Type: text/plain, Size: 1122 bytes --]

Hi Marcel,

2011/1/18 Marcel Holtmann <marcel@holtmann.org>:
>> +                             "auto"         Network registration is performed
>> +                                            automatically.
>> +                             "forced-auto"  Network registration is performed
>> +                                            automatically while manual
>> +                                            selection is disabled.
>
> we need to discuss this a little bit. I am not sure using a mode of
> forced auto is the best choice here.
>
> Maybe an extra property like "ManualAllowed" would be better. Thoughts?

The reason I ended up extending the Mode property was that once
forced_auto is true, no other Mode than auto is anyway possible. So in
a sense it is another type of Mode, which isn't to say a separate
property wouldn't work equally well.

I'm really open to either option. The way the code is written makes it
very easy to just expose this as a separate property.

Cheers,
Aki

Re: [PATCH 3/5] doc: Add documentation for 'forced-mode'

[Marcel Holtmann]

[-- Attachment #1: Type: text/plain, Size: 1583 bytes --]

Hi Aki,

> >> +                             "auto"         Network registration is performed
> >> +                                            automatically.
> >> +                             "forced-auto"  Network registration is performed
> >> +                                            automatically while manual
> >> +                                            selection is disabled.
> >
> > we need to discuss this a little bit. I am not sure using a mode of
> > forced auto is the best choice here.
> >
> > Maybe an extra property like "ManualAllowed" would be better. Thoughts?
> 
> The reason I ended up extending the Mode property was that once
> forced_auto is true, no other Mode than auto is anyway possible. So in
> a sense it is another type of Mode, which isn't to say a separate
> property wouldn't work equally well.
> 
> I'm really open to either option. The way the code is written makes it
> very easy to just expose this as a separate property.

the extra "forced-auto" mode feels kinda forced into this property. So
that is my main concern here.

However on the other hand the Mode property is actually read-only. If it
would be a user changeable property then it would be pretty clear that
we do not wanna intermix this here.

With this in mind, it might be just fine here from a UI interaction flow
point of view.

Denis, what is your take on this?

And while thinking about this, now I have another question. If this flag
is set on the SIM card, are you allowed to deregister from the network?

Regards

Marcel

Re: [PATCH 3/5] doc: Add documentation for 'forced-mode'

[Aki Niemi]

[-- Attachment #1: Type: text/plain, Size: 531 bytes --]

Hi Marcel,

2011/1/18 Marcel Holtmann <marcel@holtmann.org>:
> And while thinking about this, now I have another question. If this flag
> is set on the SIM card, are you allowed to deregister from the network?

I suppose so. However, you know how I feel about Deregister() to begin
with. I think with the Online property, it simply serves no purpose.
Or is there an actual use case for intentionally leaving RF active
while in limited service only?

For sure, you're allowed to take the device offline.

Cheers,
Aki

Re: [PATCH 3/5] doc: Add documentation for 'forced-mode'

[Marcel Holtmann]

[-- Attachment #1: Type: text/plain, Size: 690 bytes --]

Hi Aki,

> > And while thinking about this, now I have another question. If this flag
> > is set on the SIM card, are you allowed to deregister from the network?
> 
> I suppose so. However, you know how I feel about Deregister() to begin
> with. I think with the Online property, it simply serves no purpose.
> Or is there an actual use case for intentionally leaving RF active
> while in limited service only?

this is actually a good question. Do we still need the Deregister()
since we have Online property now.

I personally don't remember that we discussed this after we pushed the
Online support and fixed up the majority of the modem drivers.

Regards

Marcel

Re: [PATCH 3/5] doc: Add documentation for 'forced-mode'

[Denis Kenzior]

[-- Attachment #1: Type: text/plain, Size: 593 bytes --]

Hi Aki,

On 01/18/2011 02:35 PM, Aki Niemi wrote:
> Hi Marcel,
> 
> 2011/1/18 Marcel Holtmann <marcel@holtmann.org>:
>> And while thinking about this, now I have another question. If this flag
>> is set on the SIM card, are you allowed to deregister from the network?
> 
> I suppose so. However, you know how I feel about Deregister() to begin
> with. I think with the Online property, it simply serves no purpose.
> Or is there an actual use case for intentionally leaving RF active
> while in limited service only?
> 

I'm fine removing this one as well.

Regards,
-Denis

Re: [PATCH 3/5] doc: Add documentation for 'forced-mode'

[Denis Kenzior]

[-- Attachment #1: Type: text/plain, Size: 1084 bytes --]

Hi Marcel,

>> The reason I ended up extending the Mode property was that once
>> forced_auto is true, no other Mode than auto is anyway possible. So in
>> a sense it is another type of Mode, which isn't to say a separate
>> property wouldn't work equally well.
>>
>> I'm really open to either option. The way the code is written makes it
>> very easy to just expose this as a separate property.
> 
> the extra "forced-auto" mode feels kinda forced into this property. So
> that is my main concern here.
> 
> However on the other hand the Mode property is actually read-only. If it
> would be a user changeable property then it would be pretty clear that
> we do not wanna intermix this here.
> 
> With this in mind, it might be just fine here from a UI interaction flow
> point of view.
> 
> Denis, what is your take on this?
> 

I'm fine with forced-auto, but we might want to add a new error type to
ofono.h (maybe something like ManualRegistrationNotAllowed) and return
that error when trying to use Scan or NetworkOperator.Register.

Regards,
-Denis

Re: [PATCH 3/5] doc: Add documentation for 'forced-mode'

[Aki Niemi]

[-- Attachment #1: Type: text/plain, Size: 537 bytes --]

Hi Denis,

2011/1/18 Denis Kenzior <denkenz@gmail.com>:
> I'm fine with forced-auto, but we might want to add a new error type to
> ofono.h (maybe something like ManualRegistrationNotAllowed) and return
> that error when trying to use Scan or NetworkOperator.Register.

I was reusing the __ofono_error_access_denied(), which I think came
with the lockdown feature.
>
> Regards,
> -Denis
> _______________________________________________
> ofono mailing list
> ofono(a)ofono.org
> http://lists.ofono.org/listinfo/ofono
>

[PATCH 4/5] doc: Add short mention of CPHS under standards

[Aki Niemi]

[-- Attachment #1: Type: text/plain, Size: 801 bytes --]

---
 doc/standards.txt |    9 +++++++++
 1 files changed, 9 insertions(+), 0 deletions(-)

diff --git a/doc/standards.txt b/doc/standards.txt
index 2b85cb0..35f2a3a 100644
--- a/doc/standards.txt
+++ b/doc/standards.txt
@@ -166,3 +166,12 @@ Describes the general background and assumption of wireless features.
 	Capability: Network Perspective
 - S.R0006-808:	Wireless Features Description: Circuit Switched Call Precedence Over
 	CDMA Packet Data Session
+
+Common PCN Handset Specification (CPHS)
+=======================================
+
+This specification includes certain pre-standard extensions to GSM
+standards. oFono implements some of the features found in the Phase 2
+specification, version 4.2.
+
+The specification itself is not publicly available.
-- 
1.7.1

Re: [PATCH 4/5] doc: Add short mention of CPHS under standards

[Marcel Holtmann]

[-- Attachment #1: Type: text/plain, Size: 163 bytes --]

Hi Aki,

>  doc/standards.txt |    9 +++++++++
>  1 files changed, 9 insertions(+), 0 deletions(-)

patch has been applied. Thanks.

Regards

Marcel

[PATCH 5/5] TODO: Remove completed CPHS CSP task

[Aki Niemi]

[-- Attachment #1: Type: text/plain, Size: 909 bytes --]

---
 TODO |   13 -------------
 1 files changed, 0 insertions(+), 13 deletions(-)

diff --git a/TODO b/TODO
index 1acae81..345952e 100644
--- a/TODO
+++ b/TODO
@@ -97,19 +97,6 @@ SIM / SIM File system
   Complexity: C1
   Owner: Marit Henriksen <marit.henriksen@stericsson.com>
 
-- Add support for CPHS Customer Service Profile (CSP).  This task adds support
-  for reading the EFcsp file and in particular, interpreting the PLMN mode bit
-  within the 'Value Added Services' service group.
-
-  Based on the PLMN mode value, manual network selection needs to either be
-  enabled or disabled in the netreg atom.  Also to accommodate this, there
-  probably needs to be a new 'forced' mode to indicate that manual network
-  selection is disabled.
-
-  Priority: Medium
-  Complexity: C1
-  Owner: Aki Niemi <aki.niemi@nokia.com>
-
 Modem Emulator
 ==============
 
-- 
1.7.1