Hi Oleg,
On 12/02/2011 05:10 AM, Oleg Zhurakivskyy wrote:
---
src/network.c | 32 ++++++++++++++++++++++++--------
1 files changed, 24 insertions(+), 8 deletions(-)
diff --git a/src/network.c b/src/network.c
index 6e08876..1354edf 100644
--- a/src/network.c
+++ b/src/network.c
@@ -1719,21 +1719,37 @@ static void sim_spn_read_cb(int ok, int length, int record,
int record_length, void *user_data)
{
struct ofono_netreg *netreg = user_data;
- unsigned char dcbyte;
+ const unsigned char *dcbyte = NULL;
+ int offset = 0;
- if (!ok)
+ if (!ok) {
+ if (!(netreg->flags & NETWORK_REGISTRATION_FLAG_CPHS_SPN)) {
+
+ netreg->flags |= NETWORK_REGISTRATION_FLAG_CPHS_SPN;
+
+ ofono_sim_read(netreg->sim_context,
+ SIM_EF_CPHS_SPN_FILEID,
+ OFONO_SIM_FILE_STRUCTURE_TRANSPARENT,
+ sim_spn_read_cb, netreg);
+ }
return;
+ }
- dcbyte = data[0];
+ if (!(netreg->flags & NETWORK_REGISTRATION_FLAG_CPHS_SPN)) {
+ dcbyte = data;
+ offset = 1;
+ }
- if (!sim_spn_parse(data + 1, length - 1, &netreg->spn))
+ if (!sim_spn_parse(data + offset, length - offset, &netreg->spn))
return;
- ofono_sim_read(netreg->sim_context, SIM_EFSPDI_FILEID,
- OFONO_SIM_FILE_STRUCTURE_TRANSPARENT,
- sim_spdi_read_cb, netreg);
+ if (!(netreg->flags & NETWORK_REGISTRATION_FLAG_CPHS_SPN))
+ ofono_sim_read(netreg->sim_context, SIM_EFSPDI_FILEID,
+ OFONO_SIM_FILE_STRUCTURE_TRANSPARENT,
+ sim_spdi_read_cb, netreg);
- sim_spn_display_condition_parse(netreg, dcbyte);
+ if (dcbyte != NULL)
+ sim_spn_display_condition_parse(netreg, *dcbyte);
if (netreg->current_operator)
ofono_netreg_operator_display_name_notify(netreg);
Actually I don't like this approach. There are some interactions with
the SIM refresh logic triggered by sim_spn_changed and this approach
makes it very hard to puzzle out whether all cases are covered properly.
Also, the preference is always towards clearer, more readable code,
even if at the expense of being more verbose. Given how tiny
sim_spn_read_cb is now, I'd rather you handle the CPHS variants separately.
You are also not taking care of the CAT/SAT Refresh cases for the CPHS
variants.
Thinking more about it, the way I'd try to do this is to look up the
necessary bits in the CPHS Information Service Table (CPHS 4.2 Section
B.3.1.1). Then only read CPHS SPN if the service is active / allocated.
Only read the short form if CPHS SPN is not available and the short SPN is.
Regards,
-Denis