All of lore.kernel.org
 help / color / mirror / Atom feed
* MNC/MCC as string?
@ 2009-06-10 11:26 Aki Niemi
  2009-06-10 16:15 ` Denis Kenzior
  0 siblings, 1 reply; 26+ messages in thread
From: Aki Niemi @ 2009-06-10 11:26 UTC (permalink / raw)
  To: ofono

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


Hi,

Currently, the MNC and MCC values are of type short, which is a little
problematic. 

The MNC code can either be 2 or 3 digits, and it would be quite natural to
assume the logic is that 3 digits are used for codes > 99. However, this is
not correct -- it depends on the MCC. It seems mostly American operators
have 3 digit MNCs, whereas most of the rest of the world 2 digit MNCs. The
implication is that 01 and 001 are not considered identical.

Nokia modems both send and receive MNC/MCC pairs as Binary Coded Decimal
(BCD) strings. Any 2 digit MNC is padded with 0xF. Problem is, when listing
operators, the conversion of MNC codes from BCD to short loses this
information, and will result in manual network selection failing (BCD '001'
-> short '1' -> BCD '01F' != BCD '001').

Anyone opposed to changing the mnc and mcc code types from short to string?


Cheers,
Aki

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: MNC/MCC as string?
  2009-06-10 11:26 MNC/MCC as string? Aki Niemi
@ 2009-06-10 16:15 ` Denis Kenzior
  2009-06-10 16:42   ` =?unknown-8bit?q?R=C3=A9mi?= Denis-Courmont
  2009-06-11  6:16   ` Aki Niemi
  0 siblings, 2 replies; 26+ messages in thread
From: Denis Kenzior @ 2009-06-10 16:15 UTC (permalink / raw)
  To: ofono

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

Aki,

On Wednesday 10 June 2009 06:26:07 Aki Niemi wrote:
> Hi,
>
> Currently, the MNC and MCC values are of type short, which is a little
> problematic.
>
> The MNC code can either be 2 or 3 digits, and it would be quite natural to
> assume the logic is that 3 digits are used for codes > 99. However, this is
> not correct -- it depends on the MCC. It seems mostly American operators
> have 3 digit MNCs, whereas most of the rest of the world 2 digit MNCs. The
> implication is that 01 and 001 are not considered identical.

It doesn't seem this clear-cut.  E.g. according to my Neo on with T-Mobile US 
SIM:

AT+COPS?
+COPS: 0,0,"T-Mobile"
OK
AT+COPS=3,2
OK
AT+COPS?
+COPS: 0,2,"31026"
OK
AT+COPS=2
OK
+CREG: 0
AT+COPS=1,2,"31026"
OK
+CREG: 2
+CREG: 1,"99EC","1A11"

At least according to wikipedia the real MCC/MNC of T-Mobile is 310260.  Go 
figure.

>
> Nokia modems both send and receive MNC/MCC pairs as Binary Coded Decimal
> (BCD) strings. Any 2 digit MNC is padded with 0xF. Problem is, when listing
> operators, the conversion of MNC codes from BCD to short loses this
> information, and will result in manual network selection failing (BCD '001'
> -> short '1' -> BCD '01F' != BCD '001').
>
> Anyone opposed to changing the mnc and mcc code types from short to string?
>

I agree that this does seem to be an issue, so no problems in changing this.  
Do you consider this an implementation issue only (e.g. APIs do not change) or 
do you want to change the NetworkOperator attributes to a string as well?  If 
this is an implementation issue we can always adopt the Nokia convention of 
padding the MNC by 0xF on the right and leave them as a 'short'.

>
> Cheers,
> Aki
> _______________________________________________
> ofono mailing list
> ofono(a)ofono.org
> http://lists.ofono.org/listinfo/ofono

Regards,
-Denis

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: MNC/MCC as string?
  2009-06-10 16:15 ` Denis Kenzior
@ 2009-06-10 16:42   ` =?unknown-8bit?q?R=C3=A9mi?= Denis-Courmont
  2009-06-10 17:41     ` Denis Kenzior
  2009-06-11  6:16   ` Aki Niemi
  1 sibling, 1 reply; 26+ messages in thread
From: =?unknown-8bit?q?R=C3=A9mi?= Denis-Courmont @ 2009-06-10 16:42 UTC (permalink / raw)
  To: ofono

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

Le mercredi 10 juin 2009 19:15:19 Denis Kenzior, vous avez écrit :
> > Nokia modems both send and receive MNC/MCC pairs as Binary Coded Decimal
> > (BCD) strings. Any 2 digit MNC is padded with 0xF. Problem is, when
> > listing operators, the conversion of MNC codes from BCD to short loses
> > this information, and will result in manual network selection failing
> > (BCD '001' -> short '1' -> BCD '01F' != BCD '001').
> >
> > Anyone opposed to changing the mnc and mcc code types from short to
> > string?
>
> I agree that this does seem to be an issue, so no problems in changing
> this. Do you consider this an implementation issue only (e.g. APIs do not
> change) or do you want to change the NetworkOperator attributes to a string
> as well?  If this is an implementation issue we can always adopt the Nokia
> convention of padding the MNC by 0xF on the right and leave them as a
> 'short'.

Well, the point is that leading zeroes are meaningful (much like with phone 
numbers in fact). The API, not just the implementation, must distinguish 
network "xy" from network "0xy", so that manual selection remains unambiguous. 
A D-Bus string is probably nicer to use than an integer using ISI's BCD 
scheme.

-- 
Rémi Denis-Courmont
Nokia Devices R&D


^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: MNC/MCC as string?
  2009-06-10 16:42   ` =?unknown-8bit?q?R=C3=A9mi?= Denis-Courmont
@ 2009-06-10 17:41     ` Denis Kenzior
  2009-06-11 11:46       ` Marcel Holtmann
  0 siblings, 1 reply; 26+ messages in thread
From: Denis Kenzior @ 2009-06-10 17:41 UTC (permalink / raw)
  To: ofono

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

Hi Remi,

Well, the point is that leading zeroes are meaningful (much like with phone
> numbers in fact). The API, not just the implementation, must distinguish
> network "xy" from network "0xy", so that manual selection remains
> unambiguous.
> A D-Bus string is probably nicer to use than an integer using ISI's BCD
> scheme


I understand perfectly.  But remember, oFono does not expose the user to
such details.  Manual registration is accomplished by using Register()
method (with no arguments) of the NetworkOperator interface.  The internal
storage representation is never exposed.  Thus it doesn't matter whether
internally it is a string or short, etc.

The attributes are really only for informational purposes only.  The user
would not base his decision on the mcc/mnc, but on the operator name.

So before we start changing the D-Bus APIs, we need to answer these two
questions:
  - Can a country/administrative domain have both 001 and 01 MNCs such that
the use of string for the MNC is actually necessary.
  - What does the user find easier to use, a string or a short?

Regards,
-Denis

[-- Attachment #2: attachment.html --]
[-- Type: text/html, Size: 1392 bytes --]

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: MNC/MCC as string?
  2009-06-10 16:15 ` Denis Kenzior
  2009-06-10 16:42   ` =?unknown-8bit?q?R=C3=A9mi?= Denis-Courmont
@ 2009-06-11  6:16   ` Aki Niemi
  1 sibling, 0 replies; 26+ messages in thread
From: Aki Niemi @ 2009-06-11  6:16 UTC (permalink / raw)
  To: ofono

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


On Wed, 10 Jun 2009 11:15:19 -0500, Denis Kenzior <denkenz@gmail.com>
wrote:
> It doesn't seem this clear-cut.  E.g. according to my Neo on with
T-Mobile
> US 
> SIM:
> 
> AT+COPS?
> +COPS: 0,0,"T-Mobile"
> OK
> AT+COPS=3,2
> OK
> AT+COPS?
> +COPS: 0,2,"31026"
> OK
> AT+COPS=2
> OK
> +CREG: 0
> AT+COPS=1,2,"31026"
> OK
> +CREG: 2
> +CREG: 1,"99EC","1A11"
> 
> At least according to wikipedia the real MCC/MNC of T-Mobile is 310260. 
Go
> 
> figure.

I think this might be a separate issue with some North American operators
that seem to pad also 3-digit codes, effectively dropping that trailing
zero. Perhaps this is for legacy reasons.
 
>> Nokia modems both send and receive MNC/MCC pairs as Binary Coded Decimal
>> (BCD) strings. Any 2 digit MNC is padded with 0xF. Problem is, when
>> listing
>> operators, the conversion of MNC codes from BCD to short loses this
>> information, and will result in manual network selection failing (BCD
>> '001'
>> -> short '1' -> BCD '01F' != BCD '001').
>>
>> Anyone opposed to changing the mnc and mcc code types from short to
>> string?
>>
> 
> I agree that this does seem to be an issue, so no problems in changing
> this.  
> Do you consider this an implementation issue only (e.g. APIs do not
change)
> or 
> do you want to change the NetworkOperator attributes to a string as well?


I would go ahead and change them as well. In theory, there could exist both
2- and 3-digit MNCs within a single MCC, for which having strings is the
safest option.

Cheers,
Aki

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: MNC/MCC as string?
  2009-06-10 17:41     ` Denis Kenzior
@ 2009-06-11 11:46       ` Marcel Holtmann
  2009-06-11 14:04         ` Jan Luebbe
                           ` (2 more replies)
  0 siblings, 3 replies; 26+ messages in thread
From: Marcel Holtmann @ 2009-06-11 11:46 UTC (permalink / raw)
  To: ofono

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

Hi Denis,

> I understand perfectly.  But remember, oFono does not expose the user
> to such details.  Manual registration is accomplished by using
> Register() method (with no arguments) of the NetworkOperator
> interface.  The internal storage representation is never exposed.
> Thus it doesn't matter whether internally it is a string or short,
> etc.
> 
> The attributes are really only for informational purposes only.  The
> user would not base his decision on the mcc/mnc, but on the operator
> name.  
> 
> So before we start changing the D-Bus APIs, we need to answer these
> two questions:
>   - Can a country/administrative domain have both 001 and 01 MNCs such
> that the use of string for the MNC is actually necessary.
>   - What does the user find easier to use, a string or a short?

I wasn't aware of this and so it might be better to just expose these as
an operator id string. So we might not even split into MCC/MNC at all
since it is meaning less anyway.

That said, we do want some exposure of these values since it is an easy
way to determine geo location help and switch timezones etc. However I
am now thinking that me might just add a Country property and do the
proper translation inside oFono. Since we mostly care about these ones
most.

Regards

Marcel



^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: MNC/MCC as string?
  2009-06-11 11:46       ` Marcel Holtmann
@ 2009-06-11 14:04         ` Jan Luebbe
  2009-06-11 15:09           ` Marcel Holtmann
  2009-06-11 14:32         ` Denis Kenzior
  2009-06-11 18:10         ` Aki Niemi
  2 siblings, 1 reply; 26+ messages in thread
From: Jan Luebbe @ 2009-06-11 14:04 UTC (permalink / raw)
  To: ofono

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

On Thu, 2009-06-11 at 13:46 +0200, Marcel Holtmann wrote:
> Hi Denis,
> 
> > The attributes are really only for informational purposes only.  The
> > user would not base his decision on the mcc/mnc, but on the operator
> > name.  
> > 
> > So before we start changing the D-Bus APIs, we need to answer these
> > two questions:
> >   - Can a country/administrative domain have both 001 and 01 MNCs such
> > that the use of string for the MNC is actually necessary.
> >   - What does the user find easier to use, a string or a short?
> 
> I wasn't aware of this and so it might be better to just expose these as
> an operator id string. So we might not even split into MCC/MNC at all
> since it is meaning less anyway.

This is probably the way to go...

> That said, we do want some exposure of these values since it is an easy
> way to determine geo location help and switch timezones etc. However I
> am now thinking that me might just add a Country property and do the
> proper translation inside oFono. Since we mostly care about these ones
> most.

While inferring the country from MCC sounds nice, there are some corner
cases where is causes problems:

Digicel Bermuda uses 310/38 (or 310/038?), 310 is the US MCC:
http://dougtoombs.com/2008/07/23/bermuda-the-iphone-and-att-beware-the-data-roaming/

Montenegro used MCC 220 (Serbia) before it switched to MCC 297.

Wikipedia has a list:
http://en.wikipedia.org/wiki/Mobile_Network_Code

If you plan to ship such a list with ofono, maybe
http://git.freesmartphone.org/?p=framework.git;a=blob;f=etc/freesmartphone/ogsmd/networks.tab
can be useful for you. OTOH, Nokia probably has a better list.

Jan


^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: MNC/MCC as string?
  2009-06-11 11:46       ` Marcel Holtmann
  2009-06-11 14:04         ` Jan Luebbe
@ 2009-06-11 14:32         ` Denis Kenzior
  2009-06-11 15:12           ` Marcel Holtmann
  2009-06-11 18:00           ` Aki Niemi
  2009-06-11 18:10         ` Aki Niemi
  2 siblings, 2 replies; 26+ messages in thread
From: Denis Kenzior @ 2009-06-11 14:32 UTC (permalink / raw)
  To: ofono

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

Hi Marcel,

> I wasn't aware of this and so it might be better to just expose these as
> an operator id string. So we might not even split into MCC/MNC at all
> since it is meaning less anyway.

I'm tending to agree, which is why I wanted to start the conversation.  

By the way, just so that we're all clear, here's the relevant quote from the 
standard:

"
MCC, Mobile country code (octet 2 and 3) 
The MCC field is coded as in ITU-T Rec. E212, Annex A. 

MNC, Mobile network code (octet 3 bits 5 to 8, octet 4)  
The coding of this field is the responsibility of each administration but BCD 
coding shall be used. The MNC shall consist of 2 or 3 digits. For PCS 1900 for 
NA, Federal regulation mandates that a 3-digit MNC shall be used. However a 
network operator may decide to use only two digits in the MNC in the LAI over 
the radio interface. In this case, bits 5 to 8 of octet 3 shall be coded as 
"1111". Mobile equipment shall accept LAI coded in such a way.
"

>
> That said, we do want some exposure of these values since it is an easy
> way to determine geo location help and switch timezones etc. However I
> am now thinking that me might just add a Country property and do the
> proper translation inside oFono. Since we mostly care about these ones
> most.
>

About the only thing that MCC/MNC is useful for is to display it during manual 
operator selection.  MCC/MNC is not helpful at all for countries like U.S. 
with 4+ timezones and the same carrier id across all of them.  A country 
property would work just as well for this.  What standard do we want to use 
here? ITU 212? ISO 3166? or E164?

Any objections from actually removing the MCC/MNC Properties?

Regards,
-Denis

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: MNC/MCC as string?
  2009-06-11 14:04         ` Jan Luebbe
@ 2009-06-11 15:09           ` Marcel Holtmann
  0 siblings, 0 replies; 26+ messages in thread
From: Marcel Holtmann @ 2009-06-11 15:09 UTC (permalink / raw)
  To: ofono

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

Hi Jan,

> > > The attributes are really only for informational purposes only.  The
> > > user would not base his decision on the mcc/mnc, but on the operator
> > > name.  
> > > 
> > > So before we start changing the D-Bus APIs, we need to answer these
> > > two questions:
> > >   - Can a country/administrative domain have both 001 and 01 MNCs such
> > > that the use of string for the MNC is actually necessary.
> > >   - What does the user find easier to use, a string or a short?
> > 
> > I wasn't aware of this and so it might be better to just expose these as
> > an operator id string. So we might not even split into MCC/MNC at all
> > since it is meaning less anyway.
> 
> This is probably the way to go...
> 
> > That said, we do want some exposure of these values since it is an easy
> > way to determine geo location help and switch timezones etc. However I
> > am now thinking that me might just add a Country property and do the
> > proper translation inside oFono. Since we mostly care about these ones
> > most.
> 
> While inferring the country from MCC sounds nice, there are some corner
> cases where is causes problems:
> 
> Digicel Bermuda uses 310/38 (or 310/038?), 310 is the US MCC:
> http://dougtoombs.com/2008/07/23/bermuda-the-iphone-and-att-beware-the-data-roaming/
> 
> Montenegro used MCC 220 (Serbia) before it switched to MCC 297.
> 
> Wikipedia has a list:
> http://en.wikipedia.org/wiki/Mobile_Network_Code
> 
> If you plan to ship such a list with ofono, maybe
> http://git.freesmartphone.org/?p=framework.git;a=blob;f=etc/freesmartphone/ogsmd/networks.tab
> can be useful for you. OTOH, Nokia probably has a better list.

we will hide this minor defect inside the daemon and not bother the use
with details.

Regards

Marcel



^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: MNC/MCC as string?
  2009-06-11 14:32         ` Denis Kenzior
@ 2009-06-11 15:12           ` Marcel Holtmann
  2009-06-11 18:00           ` Aki Niemi
  1 sibling, 0 replies; 26+ messages in thread
From: Marcel Holtmann @ 2009-06-11 15:12 UTC (permalink / raw)
  To: ofono

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

Hi Denis,

> > I wasn't aware of this and so it might be better to just expose these as
> > an operator id string. So we might not even split into MCC/MNC at all
> > since it is meaning less anyway.
> 
> I'm tending to agree, which is why I wanted to start the conversation.  
> 
> By the way, just so that we're all clear, here's the relevant quote from the 
> standard:
> 
> "
> MCC, Mobile country code (octet 2 and 3) 
> The MCC field is coded as in ITU-T Rec. E212, Annex A. 
> 
> MNC, Mobile network code (octet 3 bits 5 to 8, octet 4)  
> The coding of this field is the responsibility of each administration but BCD 
> coding shall be used. The MNC shall consist of 2 or 3 digits. For PCS 1900 for 
> NA, Federal regulation mandates that a 3-digit MNC shall be used. However a 
> network operator may decide to use only two digits in the MNC in the LAI over 
> the radio interface. In this case, bits 5 to 8 of octet 3 shall be coded as 
> "1111". Mobile equipment shall accept LAI coded in such a way.
> "
> 
> >
> > That said, we do want some exposure of these values since it is an easy
> > way to determine geo location help and switch timezones etc. However I
> > am now thinking that me might just add a Country property and do the
> > proper translation inside oFono. Since we mostly care about these ones
> > most.
> >
> 
> About the only thing that MCC/MNC is useful for is to display it during manual 
> operator selection.  MCC/MNC is not helpful at all for countries like U.S. 
> with 4+ timezones and the same carrier id across all of them.  A country 
> property would work just as well for this.  What standard do we want to use 
> here? ITU 212? ISO 3166? or E164?
> 
> Any objections from actually removing the MCC/MNC Properties?

you know what. Lets remove them now. If we figure out on how to make
better use of them, we can add them later. Adding properties is way
simpler than deprecating/removing or changing them.

For the country, I would prefer simple alpha2 encoding. That way this
becomes similar to what WiFi uses for their regulatory enforcement.

Regards

Marcel



^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: MNC/MCC as string?
  2009-06-11 14:32         ` Denis Kenzior
  2009-06-11 15:12           ` Marcel Holtmann
@ 2009-06-11 18:00           ` Aki Niemi
  2009-06-11 18:10             ` Marcel Holtmann
  2009-06-11 18:24             ` Denis Kenzior
  1 sibling, 2 replies; 26+ messages in thread
From: Aki Niemi @ 2009-06-11 18:00 UTC (permalink / raw)
  To: ofono

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


On Thu, 11 Jun 2009 09:32:38 -0500, Denis Kenzior <denkenz@gmail.com>
wrote:
> About the only thing that MCC/MNC is useful for is to display it during
> manual 
> operator selection.  

That's not true. In fact, I'd say manual operator selection is just about
the last
place where the codes should be displayed.

> MCC/MNC is not helpful at all for countries like U.S. 
> with 4+ timezones and the same carrier id across all of them.  A country 
> property would work just as well for this.  

For geolocation help, sure, but there are other places where MNC/MCC are
used as keys to databases containing operator-specific information, like
default Internet APN names and such.

> What standard do we want to use
> 
> here? ITU 212? ISO 3166? or E164?
> 
> Any objections from actually removing the MCC/MNC Properties?

We need them.

Cheers,
Aki

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: MNC/MCC as string?
  2009-06-11 18:00           ` Aki Niemi
@ 2009-06-11 18:10             ` Marcel Holtmann
  2009-06-11 19:02               ` Aki Niemi
  2009-06-11 18:24             ` Denis Kenzior
  1 sibling, 1 reply; 26+ messages in thread
From: Marcel Holtmann @ 2009-06-11 18:10 UTC (permalink / raw)
  To: ofono

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

Hi Aki,

> > About the only thing that MCC/MNC is useful for is to display it during
> > manual 
> > operator selection.  
> 
> That's not true. In fact, I'd say manual operator selection is just about
> the last
> place where the codes should be displayed.
> 
> > MCC/MNC is not helpful at all for countries like U.S. 
> > with 4+ timezones and the same carrier id across all of them.  A country 
> > property would work just as well for this.  
> 
> For geolocation help, sure, but there are other places where MNC/MCC are
> used as keys to databases containing operator-specific information, like
> default Internet APN names and such.

shouldn't we just integrate that database into oFono directly or as
something like ofono-info.git that will be installed along with (for
easier updates).

I know we need this database and mobilebroadband-providerinfo or similar
project is doing this already.

Regards

Marcel



^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: MNC/MCC as string?
  2009-06-11 11:46       ` Marcel Holtmann
  2009-06-11 14:04         ` Jan Luebbe
  2009-06-11 14:32         ` Denis Kenzior
@ 2009-06-11 18:10         ` Aki Niemi
  2 siblings, 0 replies; 26+ messages in thread
From: Aki Niemi @ 2009-06-11 18:10 UTC (permalink / raw)
  To: ofono

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


On Thu, 11 Jun 2009 13:46:55 +0200, Marcel Holtmann <marcel@holtmann.org>
wrote:
> That said, we do want some exposure of these values since it is an easy
> way to determine geo location help and switch timezones etc. 

For geolocation, cell ID and LAC are more useful, especially coupled with
information on neighboring cells, RTT measurements, etc. We will probably
need to expose this information some way.

Also, typically networks provide date-time, timezone and DST information
via NITZ [1].

Cheers,
Aki

[1] http://en.wikipedia.org/wiki/NITZ

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: MNC/MCC as string?
  2009-06-11 18:00           ` Aki Niemi
  2009-06-11 18:10             ` Marcel Holtmann
@ 2009-06-11 18:24             ` Denis Kenzior
  1 sibling, 0 replies; 26+ messages in thread
From: Denis Kenzior @ 2009-06-11 18:24 UTC (permalink / raw)
  To: ofono

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

Aki,

On Thursday 11 June 2009 13:00:48 Aki Niemi wrote:
> On Thu, 11 Jun 2009 09:32:38 -0500, Denis Kenzior <denkenz@gmail.com>
>
> wrote:
> > About the only thing that MCC/MNC is useful for is to display it during
> > manual
> > operator selection.
>
> That's not true. In fact, I'd say manual operator selection is just about
> the last
> place where the codes should be displayed.

The point is MCCMNC is not something the user should realy be exposed to.  It 
is useful to dis-ambiguate the carrier by Country when you're in a border 
area, but that is about it. Country property can do this just as well.

>
> > MCC/MNC is not helpful at all for countries like U.S.
> > with 4+ timezones and the same carrier id across all of them.  A country
> > property would work just as well for this.
>
> For geolocation help, sure, but there are other places where MNC/MCC are
> used as keys to databases containing operator-specific information, like
> default Internet APN names and such.
>

Then lets design this into oFono instead of exposing an obscure attribute.

Regards,
-Denis

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: MNC/MCC as string?
  2009-06-11 18:10             ` Marcel Holtmann
@ 2009-06-11 19:02               ` Aki Niemi
  2009-06-11 19:18                 ` Marcel Holtmann
  2009-06-11 20:35                 ` Marcel Holtmann
  0 siblings, 2 replies; 26+ messages in thread
From: Aki Niemi @ 2009-06-11 19:02 UTC (permalink / raw)
  To: ofono

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


On Thu, 11 Jun 2009 20:10:27 +0200, Marcel Holtmann <marcel@holtmann.org>
wrote:
>> For geolocation help, sure, but there are other places where MNC/MCC are
>> used as keys to databases containing operator-specific information, like
>> default Internet APN names and such.
> 
> shouldn't we just integrate that database into oFono directly or as
> something like ofono-info.git that will be installed along with (for
> easier updates).

This particular database of default APN names, maybe, although I think it's
really oFono users like Connman that should integrate with that
information. 

However, this wouldn't be the only database ever, so we would still need a
way to reliably identify a specific operator.

Cheers,
Aki

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: MNC/MCC as string?
  2009-06-11 19:02               ` Aki Niemi
@ 2009-06-11 19:18                 ` Marcel Holtmann
  2009-06-11 19:38                   ` Aki Niemi
  2009-06-11 19:58                   ` =?unknown-8bit?q?R=C3=A9mi?= Denis-Courmont
  2009-06-11 20:35                 ` Marcel Holtmann
  1 sibling, 2 replies; 26+ messages in thread
From: Marcel Holtmann @ 2009-06-11 19:18 UTC (permalink / raw)
  To: ofono

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

Hi Aki,

> >> For geolocation help, sure, but there are other places where MNC/MCC are
> >> used as keys to databases containing operator-specific information, like
> >> default Internet APN names and such.
> > 
> > shouldn't we just integrate that database into oFono directly or as
> > something like ofono-info.git that will be installed along with (for
> > easier updates).
> 
> This particular database of default APN names, maybe, although I think it's
> really oFono users like Connman that should integrate with that
> information. 
> 
> However, this wouldn't be the only database ever, so we would still need a
> way to reliably identify a specific operator.

if we are unclear about the format and the users at this moment, I
prefer we remove that interface and just keep it around as a TODO item
to add it later.

As I said, adding something is easy, removing/deprecating hard and
changing the format of it even harder.

Regards

Marcel



^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: MNC/MCC as string?
  2009-06-11 19:18                 ` Marcel Holtmann
@ 2009-06-11 19:38                   ` Aki Niemi
  2009-06-11 20:08                     ` Denis Kenzior
  2009-06-11 19:58                   ` =?unknown-8bit?q?R=C3=A9mi?= Denis-Courmont
  1 sibling, 1 reply; 26+ messages in thread
From: Aki Niemi @ 2009-06-11 19:38 UTC (permalink / raw)
  To: ofono

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


On Thu, 11 Jun 2009 21:18:02 +0200, Marcel Holtmann <marcel@holtmann.org>
wrote:
>> However, this wouldn't be the only database ever, so we would still need
>> a
>> way to reliably identify a specific operator.
> 
> if we are unclear about the format and the users at this moment,

The format should be 's' in D-Bus and in struct ofono_network_operator:

   char mnc[4];
   char mcc[4];

And as for users, we need access to both MNC and MCC.

What part of this is unclear?

Cheers,
Aki

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: MNC/MCC as string?
  2009-06-11 19:18                 ` Marcel Holtmann
  2009-06-11 19:38                   ` Aki Niemi
@ 2009-06-11 19:58                   ` =?unknown-8bit?q?R=C3=A9mi?= Denis-Courmont
  2009-06-11 20:33                     ` Marcel Holtmann
  1 sibling, 1 reply; 26+ messages in thread
From: =?unknown-8bit?q?R=C3=A9mi?= Denis-Courmont @ 2009-06-11 19:58 UTC (permalink / raw)
  To: ofono

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

Le jeudi 11 juin 2009 22:18:02 Marcel Holtmann, vous avez écrit :
> Hi Aki,
>
> > >> For geolocation help, sure, but there are other places where MNC/MCC
> > >> are used as keys to databases containing operator-specific
> > >> information, like default Internet APN names and such.
> > >
> > > shouldn't we just integrate that database into oFono directly or as
> > > something like ofono-info.git that will be installed along with (for
> > > easier updates).
> >
> > This particular database of default APN names, maybe, although I think
> > it's really oFono users like Connman that should integrate with that
> > information.
> >
> > However, this wouldn't be the only database ever, so we would still need
> > a way to reliably identify a specific operator.
>
> if we are unclear about the format and the users at this moment, I
> prefer we remove that interface and just keep it around as a TODO item
> to add it later.

I don't see any lack of clarity. It is a series of two or three digits.

Anyway, if the database has a hole, we probably will have to return the 
MNC/MCC anyway (ever tried scanning for network with an old phone in a foreign 
country?). Then there is also the uniqueness problem. MNC/MCC pair solves 
that.

-- 
Rémi Denis-Courmont
http://www.remlab.net/


^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: MNC/MCC as string?
  2009-06-11 19:38                   ` Aki Niemi
@ 2009-06-11 20:08                     ` Denis Kenzior
  2009-06-12 10:01                       ` Aki Niemi
  0 siblings, 1 reply; 26+ messages in thread
From: Denis Kenzior @ 2009-06-11 20:08 UTC (permalink / raw)
  To: ofono

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

Aki,

On Thursday 11 June 2009 14:38:49 Aki Niemi wrote:
> On Thu, 11 Jun 2009 21:18:02 +0200, Marcel Holtmann <marcel@holtmann.org>
>
> wrote:
> >> However, this wouldn't be the only database ever, so we would still need
> >> a
> >> way to reliably identify a specific operator.
> >
> > if we are unclear about the format and the users at this moment,
>
> The format should be 's' in D-Bus and in struct ofono_network_operator:
>
>    char mnc[4];
>    char mcc[4];
>
> And as for users, we need access to both MNC and MCC.
>
> What part of this is unclear?

Please submit a patch to the mailing list reflecting how you think it should be 
implemented.  If it looks reasonable then I'll integrate it.

Regards,
-Denis


^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: MNC/MCC as string?
  2009-06-11 19:58                   ` =?unknown-8bit?q?R=C3=A9mi?= Denis-Courmont
@ 2009-06-11 20:33                     ` Marcel Holtmann
  0 siblings, 0 replies; 26+ messages in thread
From: Marcel Holtmann @ 2009-06-11 20:33 UTC (permalink / raw)
  To: ofono

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

Hi Remi,

> > > >> For geolocation help, sure, but there are other places where MNC/MCC
> > > >> are used as keys to databases containing operator-specific
> > > >> information, like default Internet APN names and such.
> > > >
> > > > shouldn't we just integrate that database into oFono directly or as
> > > > something like ofono-info.git that will be installed along with (for
> > > > easier updates).
> > >
> > > This particular database of default APN names, maybe, although I think
> > > it's really oFono users like Connman that should integrate with that
> > > information.
> > >
> > > However, this wouldn't be the only database ever, so we would still need
> > > a way to reliably identify a specific operator.
> >
> > if we are unclear about the format and the users at this moment, I
> > prefer we remove that interface and just keep it around as a TODO item
> > to add it later.
> 
> I don't see any lack of clarity. It is a series of two or three digits.
> 
> Anyway, if the database has a hole, we probably will have to return the 
> MNC/MCC anyway (ever tried scanning for network with an old phone in a foreign 
> country?). Then there is also the uniqueness problem. MNC/MCC pair solves 
> that.

if the database has a whole or we don't have the network provider string
then we can just show the MNC/MCC in that case. That is pretty simple.
No need to put extra burden into the userspace application and have it
looking for two strings.

I don't understand your uniqueness issue. We will have unique object
path for every network. What else does userspace needs?

Regards

Marcel



^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: MNC/MCC as string?
  2009-06-11 19:02               ` Aki Niemi
  2009-06-11 19:18                 ` Marcel Holtmann
@ 2009-06-11 20:35                 ` Marcel Holtmann
  1 sibling, 0 replies; 26+ messages in thread
From: Marcel Holtmann @ 2009-06-11 20:35 UTC (permalink / raw)
  To: ofono

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

Hi Aki,

> >> For geolocation help, sure, but there are other places where MNC/MCC are
> >> used as keys to databases containing operator-specific information, like
> >> default Internet APN names and such.
> > 
> > shouldn't we just integrate that database into oFono directly or as
> > something like ofono-info.git that will be installed along with (for
> > easier updates).
> 
> This particular database of default APN names, maybe, although I think it's
> really oFono users like Connman that should integrate with that
> information. 
> 
> However, this wouldn't be the only database ever, so we would still need a
> way to reliably identify a specific operator.

I agree that there might be at some point a need for it, but what I
question is the need for it right now. Do we have direct users in
userspace that can make use of this information? Giving them directly to
the user makes no sense.

Regards

Marcel



^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: MNC/MCC as string?
  2009-06-11 20:08                     ` Denis Kenzior
@ 2009-06-12 10:01                       ` Aki Niemi
  2009-06-12 10:12                         ` Marcel Holtmann
  0 siblings, 1 reply; 26+ messages in thread
From: Aki Niemi @ 2009-06-12 10:01 UTC (permalink / raw)
  To: ofono

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

On Thu, 11 Jun 2009 15:08:08 -0500, Denis Kenzior <denkenz@gmail.com>
wrote:
>> What part of this is unclear?
> 
> Please submit a patch to the mailing list reflecting how you think it
> should be 
> implemented.  If it looks reasonable then I'll integrate it.

Uh. You know I am one of the co-maintainers here? ;)

Anyway, here it is. Please test; seems to work for me on my N95.

Cheers,
Aki

[-- Attachment #2: 0001-Change-MNC-and-MCC-variable-types-to-string.patch --]
[-- Type: text/plain, Size: 8196 bytes --]

From 88e6a0a521c69913ab954e5a79f923b317d5ed26 Mon Sep 17 00:00:00 2001
From: Aki Niemi <aki.niemi@nokia.com>
Date: Fri, 12 Jun 2009 10:02:52 +0300
Subject: [PATCH] Change MNC and MCC variable types to string

This is to make sure both 2 and 3-digit MNC values are correctly
handled. Both the modem plugin API as well as the D-Bus API are
affected.
---
 drivers/atmodem/network-registration.c |   51 +++++++++++++------------------
 src/driver.h                           |    4 +-
 src/network.c                          |   48 ++++++++++++------------------
 3 files changed, 42 insertions(+), 61 deletions(-)

diff --git a/drivers/atmodem/network-registration.c b/drivers/atmodem/network-registration.c
index 40796d5..f73bb7c 100644
--- a/drivers/atmodem/network-registration.c
+++ b/drivers/atmodem/network-registration.c
@@ -45,28 +45,18 @@ static const char *csq_prefix[] = { "+CSQ:", NULL };
 
 struct netreg_data {
 	gboolean supports_tech;
-	short mnc;
-	short mcc;
+	char mnc[4];
+	char mcc[4];
 };
 
-static void extract_mcc_mnc(const char *str, short *mcc, short *mnc)
+static void extract_mcc_mnc(const char *str, char *mcc, char *mnc)
 {
-	int num = 0;
-	unsigned int i;
-
 	/* Three digit country code */
-	for (i = 0; i < 3; i++)
-		num = num * 10 + (int)(str[i] - '0');
-
-	*mcc = num;
-
-	num = 0;
+	strncpy(mcc, str, sizeof(mcc));
+	mcc[3] = '\0';
 
 	/* Usually a 2 but sometimes 3 digit network code */
-	for (; i < strlen(str); i++)
-		num = num * 10 + (int)(str[i] - '0');
-
-	*mnc = num;
+	strncpy(mnc, str + 3, sizeof(mnc));
 }
 
 static void at_creg_cb(gboolean ok, GAtResult *result, gpointer user_data)
@@ -154,7 +144,7 @@ static void cops_cb(gboolean ok, GAtResult *result, gpointer user_data)
 	dump_response("cops_cb", ok, result);
 	decode_at_error(&error, g_at_result_final_response(result));
 
-	if (!ok || at->netreg->mcc == -1 || at->netreg->mnc == -1) {
+	if (!ok || *at->netreg->mcc == '\0' || *at->netreg->mnc == '\0') {
 		cb(&error, NULL, cbd->data);
 		goto out;
 	}
@@ -181,12 +171,12 @@ static void cops_cb(gboolean ok, GAtResult *result, gpointer user_data)
 	strncpy(op.name, name, OFONO_MAX_OPERATOR_NAME_LENGTH);
 	op.name[OFONO_MAX_OPERATOR_NAME_LENGTH] = '\0';
 
-	op.mcc = at->netreg->mcc;
-	op.mnc = at->netreg->mnc;
+	strncpy(op.mcc, at->netreg->mcc, sizeof(op.mcc));
+	strncpy(op.mnc, at->netreg->mnc, sizeof(op.mnc));
 	op.status = -1;
 	op.tech = tech;
 
-	ofono_debug("cops_cb: %s, %hd %hd %d", name, at->netreg->mcc,
+	ofono_debug("cops_cb: %s, %s %s %d", name, at->netreg->mcc,
 			at->netreg->mnc, tech);
 
 	cb(&error, &op, cbd->data);
@@ -235,15 +225,16 @@ static void cops_numeric_cb(gboolean ok, GAtResult *result, gpointer user_data)
 		strlen(str) == 0)
 		goto error;
 
-	extract_mcc_mnc(str, &at->netreg->mcc, &at->netreg->mnc);
+	extract_mcc_mnc(str, at->netreg->mcc, at->netreg->mnc);
 
-	ofono_debug("Cops numeric got mcc: %hd, mnc: %hd",
+	ofono_debug("Cops numeric got mcc: %s, mnc: %s",
 			at->netreg->mcc, at->netreg->mnc);
 
 	return;
 
 error:
-	at->netreg->mcc = at->netreg->mnc = -1;
+	*at->netreg->mcc = '\0';
+	*at->netreg->mnc = '\0';
 }
 
 static void at_current_operator(struct ofono_modem *modem,
@@ -356,7 +347,7 @@ static void cops_list_cb(gboolean ok, GAtResult *result, gpointer user_data)
 			if (!g_at_result_iter_next_string(&iter, &n))
 				break;
 
-			extract_mcc_mnc(n, &list[num].mcc, &list[num].mnc);
+			extract_mcc_mnc(n, list[num].mcc, list[num].mnc);
 
 			if (!g_at_result_iter_next_number(&iter, &tech))
 				tech = 0;
@@ -376,7 +367,7 @@ static void cops_list_cb(gboolean ok, GAtResult *result, gpointer user_data)
 	int i = 0;
 
 	for (; i < num; i++) {
-		ofono_debug("Operator: %s, %hd, %hd, status: %d, %d",
+		ofono_debug("Operator: %s, %s, %s, status: %d, %d",
 				list[i].name, list[i].mcc, list[i].mnc,
 				list[i].status, list[i].tech);
 	}
@@ -457,12 +448,12 @@ static void at_register_manual(struct ofono_modem *modem,
 		goto error;
 
 	if (at->netreg->supports_tech && oper->tech != -1)
-		sprintf(buf, "AT+COPS=1,2,\"%03hd%02hd\",%1d", oper->mcc,
-							oper->mnc,
-							oper->tech);
+		sprintf(buf, "AT+COPS=1,2,\"%s%s\",%1d", oper->mcc,
+						oper->mnc,
+						oper->tech);
 	else
-		sprintf(buf, "AT+COPS=1,2,\"%03hd%02hd\"", oper->mcc,
-							oper->mnc);
+		sprintf(buf, "AT+COPS=1,2,\"%s%s\"", oper->mcc,
+						oper->mnc);
 
 	if (g_at_chat_send(at->parser, buf, none_prefix,
 				register_cb, cbd, g_free) > 0)
diff --git a/src/driver.h b/src/driver.h
index 61504dd..33b7586 100644
--- a/src/driver.h
+++ b/src/driver.h
@@ -76,8 +76,8 @@ struct ofono_call {
 
 struct ofono_network_operator {
 	char name[OFONO_MAX_OPERATOR_NAME_LENGTH + 1];
-	short mcc;
-	short mnc;
+	char mcc[4];
+	char mnc[4];
 	int status;
 	int tech;
 };
diff --git a/src/network.c b/src/network.c
index f797165..aa13077 100644
--- a/src/network.c
+++ b/src/network.c
@@ -180,8 +180,9 @@ static void network_operator_populate_registered(struct ofono_modem *modem,
 	int modem_len;
 	int num_children;
 	GSList *l;
-	int *mccmnc;
 	char path[MAX_DBUS_PATH_LEN];
+	char mnc[4];
+	char mcc[4];
 
 	modem_len = snprintf(path, MAX_DBUS_PATH_LEN, "%s/operator",
 				modem->path);
@@ -199,10 +200,6 @@ static void network_operator_populate_registered(struct ofono_modem *modem,
 
 	*network_operators = g_try_new0(char *, num_children + 1);
 
-	mccmnc = g_try_new0(int, num_children * 2);
-	for (i = 0; i < num_children; i++)
-		sscanf(children[i], "%3d%3d", &mccmnc[i*2], &mccmnc[i*2+1]);
-
 	/* Quoting 27.007: "The list of operators shall be in order: home
 	 * network, networks referenced in SIM or active application in the
 	 * UICC (GSM or USIM) in the following order: HPLMN selector, User
@@ -217,18 +214,17 @@ static void network_operator_populate_registered(struct ofono_modem *modem,
 		int j;
 
 		for (j = 0; children[j]; j++) {
-			if (op->mcc == mccmnc[j*2] && op->mnc == mccmnc[j*2+1]) {
-				/* Enough to store '/' + 3 char wide MCC + 3 char wide MNC + null */
-				(*network_operators)[i] =  g_try_new(char, modem_len + 8);
-				snprintf((*network_operators)[i], modem_len + 8, "%s/%s",
+			sscanf(children[j], "%3[0-9]%[0-9]", mcc, mnc);
+			if (strcmp(op->mcc, mcc) == 0 && strcmp(op->mnc, mnc) == 0) {
+				/* Enough to store '/' + MCC + '_' + MNC + null */
+				(*network_operators)[i] =  g_try_new(char, modem_len + 9);
+				snprintf((*network_operators)[i], modem_len + 9, "%s/%s",
 					path, children[j]);
 				++i;
 			}
 		}
 	}
 
-	g_free(mccmnc);
-
 	dbus_free_string_array(children);
 }
 
@@ -244,19 +240,13 @@ static gint network_operator_compare(gconstpointer a, gconstpointer b)
 	const struct ofono_network_operator *opa = a;
 	const struct ofono_network_operator *opb = b;
 
-	if (opa->mcc < opb->mcc)
-		return -1;
+	int comp1;
+	int comp2;
 
-	if (opa->mcc > opb->mcc)
-		return 1;
+	comp1 = strcmp(opa->mcc, opb->mcc);
+	comp2 = strcmp(opa->mnc, opb->mnc);
 
-	if (opa->mnc < opb->mnc)
-		return -1;
-
-	if (opa->mnc > opb->mnc)
-		return 1;
-
-	return 0;
+	return comp1 != 0 ? comp1 : comp2;
 }
 
 static inline const char *network_operator_build_path(struct ofono_modem *modem,
@@ -264,7 +254,7 @@ static inline const char *network_operator_build_path(struct ofono_modem *modem,
 {
 	static char path[MAX_DBUS_PATH_LEN];
 
-	snprintf(path, MAX_DBUS_PATH_LEN, "%s/operator/%03d%03d",
+	snprintf(path, MAX_DBUS_PATH_LEN, "%s/operator/%s%s",
 			modem->path, oper->mcc, oper->mnc);
 
 	return path;
@@ -427,16 +417,16 @@ static DBusMessage *network_operator_get_properties(DBusConnection *conn,
 
 	dbus_gsm_dict_append(&dict, "Status", DBUS_TYPE_STRING, &status);
 
-	if (op->operator->mcc != -1) {
-		dbus_uint16_t mcc = op->operator->mcc;
+	if (*op->operator->mcc != '\0') {
+		const char *mcc = op->operator->mcc;
 		dbus_gsm_dict_append(&dict, "MobileCountryCode",
-					DBUS_TYPE_UINT16, &mcc);
+					DBUS_TYPE_STRING, &mcc);
 	}
 
-	if (op->operator->mnc != -1) {
-		dbus_uint16_t mnc = op->operator->mnc;
+	if (*op->operator->mnc != '\0') {
+		const char *mnc = op->operator->mnc;
 		dbus_gsm_dict_append(&dict, "MobileNetworkCode",
-					DBUS_TYPE_UINT16, &mnc);
+					DBUS_TYPE_STRING, &mnc);
 	}
 
 	if (op->operator->tech != -1) {
-- 
1.6.0.4


^ permalink raw reply related	[flat|nested] 26+ messages in thread

* Re: MNC/MCC as string?
  2009-06-12 10:01                       ` Aki Niemi
@ 2009-06-12 10:12                         ` Marcel Holtmann
  2009-06-12 10:53                           ` Aki Niemi
  2009-06-12 10:56                           ` =?unknown-8bit?q?R=C3=A9mi?= Denis-Courmont
  0 siblings, 2 replies; 26+ messages in thread
From: Marcel Holtmann @ 2009-06-12 10:12 UTC (permalink / raw)
  To: ofono

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

Hi Aki,

> Anyway, here it is. Please test; seems to work for me on my N95.

+static void extract_mcc_mnc(const char *str, char *mcc, char *mnc)
 {
-       int num = 0;
-       unsigned int i;
-
        /* Three digit country code */
-       for (i = 0; i < 3; i++)
-               num = num * 10 + (int)(str[i] - '0');
-
-       *mcc = num;
-
-       num = 0;
+       strncpy(mcc, str, sizeof(mcc));
+       mcc[3] = '\0';

maybe I am blind, but how is this suppose to work. The sizeof(mmc) is 1
byte.

Regards

Marcel



^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: MNC/MCC as string?
  2009-06-12 10:12                         ` Marcel Holtmann
@ 2009-06-12 10:53                           ` Aki Niemi
  2009-06-13  1:01                             ` Denis Kenzior
  2009-06-12 10:56                           ` =?unknown-8bit?q?R=C3=A9mi?= Denis-Courmont
  1 sibling, 1 reply; 26+ messages in thread
From: Aki Niemi @ 2009-06-12 10:53 UTC (permalink / raw)
  To: ofono

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

On Fri, 12 Jun 2009 12:12:33 +0200, Marcel Holtmann <marcel@holtmann.org>
wrote:
> +       strncpy(mcc, str, sizeof(mcc));
> +       mcc[3] = '\0';
> 
> maybe I am blind, but how is this suppose to work. The sizeof(mmc) is 1
> byte.

Actually, sizeof(mmc) is 4 bytes. Coincidentally, so is the char array in
mcc. ;)

Take two attached.

Cheers,
Aki

[-- Attachment #2: 0001-Change-MNC-and-MCC-variable-types-to-string.patch --]
[-- Type: text/plain, Size: 8689 bytes --]

From 5ea55baef4a10b3977675284c83d21e0a8f54a7b Mon Sep 17 00:00:00 2001
From: Aki Niemi <aki.niemi@nokia.com>
Date: Fri, 12 Jun 2009 10:02:52 +0300
Subject: [PATCH] Change MNC and MCC variable types to string

This is to make sure both 2 and 3-digit MNC values are correctly
handled. Both the modem plugin API as well as the D-Bus API are
affected.
---
 drivers/atmodem/network-registration.c |   57 +++++++++++++++-----------------
 src/driver.h                           |    7 +++-
 src/network.c                          |   48 ++++++++++----------------
 3 files changed, 51 insertions(+), 61 deletions(-)

diff --git a/drivers/atmodem/network-registration.c b/drivers/atmodem/network-registration.c
index 40796d5..30dd0f7 100644
--- a/drivers/atmodem/network-registration.c
+++ b/drivers/atmodem/network-registration.c
@@ -45,28 +45,20 @@ static const char *csq_prefix[] = { "+CSQ:", NULL };
 
 struct netreg_data {
 	gboolean supports_tech;
-	short mnc;
-	short mcc;
+	char mnc[OFONO_MAX_MNC_MCC_LENGTH + 1];
+	char mcc[OFONO_MAX_MNC_MCC_LENGTH + 1];
 };
 
-static void extract_mcc_mnc(const char *str, short *mcc, short *mnc)
+static void extract_mcc_mnc(const char *str, char *mcc, char *mnc)
 {
-	int num = 0;
-	unsigned int i;
-
 	/* Three digit country code */
-	for (i = 0; i < 3; i++)
-		num = num * 10 + (int)(str[i] - '0');
-
-	*mcc = num;
-
-	num = 0;
+	strncpy(mcc, str, OFONO_MAX_MNC_MCC_LENGTH);
+	mcc[OFONO_MAX_MNC_MCC_LENGTH] = '\0';
 
 	/* Usually a 2 but sometimes 3 digit network code */
-	for (; i < strlen(str); i++)
-		num = num * 10 + (int)(str[i] - '0');
-
-	*mnc = num;
+	strncpy(mnc, str + OFONO_MAX_MNC_MCC_LENGTH,
+		OFONO_MAX_MNC_MCC_LENGTH);
+	mnc[OFONO_MAX_MNC_MCC_LENGTH] = '\0';
 }
 
 static void at_creg_cb(gboolean ok, GAtResult *result, gpointer user_data)
@@ -154,7 +146,7 @@ static void cops_cb(gboolean ok, GAtResult *result, gpointer user_data)
 	dump_response("cops_cb", ok, result);
 	decode_at_error(&error, g_at_result_final_response(result));
 
-	if (!ok || at->netreg->mcc == -1 || at->netreg->mnc == -1) {
+	if (!ok || *at->netreg->mcc == '\0' || *at->netreg->mnc == '\0') {
 		cb(&error, NULL, cbd->data);
 		goto out;
 	}
@@ -181,12 +173,16 @@ static void cops_cb(gboolean ok, GAtResult *result, gpointer user_data)
 	strncpy(op.name, name, OFONO_MAX_OPERATOR_NAME_LENGTH);
 	op.name[OFONO_MAX_OPERATOR_NAME_LENGTH] = '\0';
 
-	op.mcc = at->netreg->mcc;
-	op.mnc = at->netreg->mnc;
+	strncpy(op.mcc, at->netreg->mcc, OFONO_MAX_MNC_MCC_LENGTH);
+	op.mcc[OFONO_MAX_MNC_MCC_LENGTH] = '\0';
+
+	strncpy(op.mnc, at->netreg->mnc, OFONO_MAX_MNC_MCC_LENGTH);
+	op.mnc[OFONO_MAX_MNC_MCC_LENGTH] = '\0';
+
 	op.status = -1;
 	op.tech = tech;
 
-	ofono_debug("cops_cb: %s, %hd %hd %d", name, at->netreg->mcc,
+	ofono_debug("cops_cb: %s, %s %s %d", name, at->netreg->mcc,
 			at->netreg->mnc, tech);
 
 	cb(&error, &op, cbd->data);
@@ -235,15 +231,16 @@ static void cops_numeric_cb(gboolean ok, GAtResult *result, gpointer user_data)
 		strlen(str) == 0)
 		goto error;
 
-	extract_mcc_mnc(str, &at->netreg->mcc, &at->netreg->mnc);
+	extract_mcc_mnc(str, at->netreg->mcc, at->netreg->mnc);
 
-	ofono_debug("Cops numeric got mcc: %hd, mnc: %hd",
+	ofono_debug("Cops numeric got mcc: %s, mnc: %s",
 			at->netreg->mcc, at->netreg->mnc);
 
 	return;
 
 error:
-	at->netreg->mcc = at->netreg->mnc = -1;
+	*at->netreg->mcc = '\0';
+	*at->netreg->mnc = '\0';
 }
 
 static void at_current_operator(struct ofono_modem *modem,
@@ -356,7 +353,7 @@ static void cops_list_cb(gboolean ok, GAtResult *result, gpointer user_data)
 			if (!g_at_result_iter_next_string(&iter, &n))
 				break;
 
-			extract_mcc_mnc(n, &list[num].mcc, &list[num].mnc);
+			extract_mcc_mnc(n, list[num].mcc, list[num].mnc);
 
 			if (!g_at_result_iter_next_number(&iter, &tech))
 				tech = 0;
@@ -376,7 +373,7 @@ static void cops_list_cb(gboolean ok, GAtResult *result, gpointer user_data)
 	int i = 0;
 
 	for (; i < num; i++) {
-		ofono_debug("Operator: %s, %hd, %hd, status: %d, %d",
+		ofono_debug("Operator: %s, %s, %s, status: %d, %d",
 				list[i].name, list[i].mcc, list[i].mnc,
 				list[i].status, list[i].tech);
 	}
@@ -457,12 +454,12 @@ static void at_register_manual(struct ofono_modem *modem,
 		goto error;
 
 	if (at->netreg->supports_tech && oper->tech != -1)
-		sprintf(buf, "AT+COPS=1,2,\"%03hd%02hd\",%1d", oper->mcc,
-							oper->mnc,
-							oper->tech);
+		sprintf(buf, "AT+COPS=1,2,\"%s%s\",%1d", oper->mcc,
+						oper->mnc,
+						oper->tech);
 	else
-		sprintf(buf, "AT+COPS=1,2,\"%03hd%02hd\"", oper->mcc,
-							oper->mnc);
+		sprintf(buf, "AT+COPS=1,2,\"%s%s\"", oper->mcc,
+						oper->mnc);
 
 	if (g_at_chat_send(at->parser, buf, none_prefix,
 				register_cb, cbd, g_free) > 0)
diff --git a/src/driver.h b/src/driver.h
index 61504dd..f753c7f 100644
--- a/src/driver.h
+++ b/src/driver.h
@@ -74,10 +74,13 @@ struct ofono_call {
  *   */
 #define OFONO_MAX_OPERATOR_NAME_LENGTH 63
 
+/* MCC is always three digits. MNC is either two or three digits */
+#define OFONO_MAX_MNC_MCC_LENGTH 3
+
 struct ofono_network_operator {
 	char name[OFONO_MAX_OPERATOR_NAME_LENGTH + 1];
-	short mcc;
-	short mnc;
+	char mcc[OFONO_MAX_MNC_MCC_LENGTH + 1];
+	char mnc[OFONO_MAX_MNC_MCC_LENGTH + 1];
 	int status;
 	int tech;
 };
diff --git a/src/network.c b/src/network.c
index f797165..aa13077 100644
--- a/src/network.c
+++ b/src/network.c
@@ -180,8 +180,9 @@ static void network_operator_populate_registered(struct ofono_modem *modem,
 	int modem_len;
 	int num_children;
 	GSList *l;
-	int *mccmnc;
 	char path[MAX_DBUS_PATH_LEN];
+	char mnc[4];
+	char mcc[4];
 
 	modem_len = snprintf(path, MAX_DBUS_PATH_LEN, "%s/operator",
 				modem->path);
@@ -199,10 +200,6 @@ static void network_operator_populate_registered(struct ofono_modem *modem,
 
 	*network_operators = g_try_new0(char *, num_children + 1);
 
-	mccmnc = g_try_new0(int, num_children * 2);
-	for (i = 0; i < num_children; i++)
-		sscanf(children[i], "%3d%3d", &mccmnc[i*2], &mccmnc[i*2+1]);
-
 	/* Quoting 27.007: "The list of operators shall be in order: home
 	 * network, networks referenced in SIM or active application in the
 	 * UICC (GSM or USIM) in the following order: HPLMN selector, User
@@ -217,18 +214,17 @@ static void network_operator_populate_registered(struct ofono_modem *modem,
 		int j;
 
 		for (j = 0; children[j]; j++) {
-			if (op->mcc == mccmnc[j*2] && op->mnc == mccmnc[j*2+1]) {
-				/* Enough to store '/' + 3 char wide MCC + 3 char wide MNC + null */
-				(*network_operators)[i] =  g_try_new(char, modem_len + 8);
-				snprintf((*network_operators)[i], modem_len + 8, "%s/%s",
+			sscanf(children[j], "%3[0-9]%[0-9]", mcc, mnc);
+			if (strcmp(op->mcc, mcc) == 0 && strcmp(op->mnc, mnc) == 0) {
+				/* Enough to store '/' + MCC + '_' + MNC + null */
+				(*network_operators)[i] =  g_try_new(char, modem_len + 9);
+				snprintf((*network_operators)[i], modem_len + 9, "%s/%s",
 					path, children[j]);
 				++i;
 			}
 		}
 	}
 
-	g_free(mccmnc);
-
 	dbus_free_string_array(children);
 }
 
@@ -244,19 +240,13 @@ static gint network_operator_compare(gconstpointer a, gconstpointer b)
 	const struct ofono_network_operator *opa = a;
 	const struct ofono_network_operator *opb = b;
 
-	if (opa->mcc < opb->mcc)
-		return -1;
+	int comp1;
+	int comp2;
 
-	if (opa->mcc > opb->mcc)
-		return 1;
+	comp1 = strcmp(opa->mcc, opb->mcc);
+	comp2 = strcmp(opa->mnc, opb->mnc);
 
-	if (opa->mnc < opb->mnc)
-		return -1;
-
-	if (opa->mnc > opb->mnc)
-		return 1;
-
-	return 0;
+	return comp1 != 0 ? comp1 : comp2;
 }
 
 static inline const char *network_operator_build_path(struct ofono_modem *modem,
@@ -264,7 +254,7 @@ static inline const char *network_operator_build_path(struct ofono_modem *modem,
 {
 	static char path[MAX_DBUS_PATH_LEN];
 
-	snprintf(path, MAX_DBUS_PATH_LEN, "%s/operator/%03d%03d",
+	snprintf(path, MAX_DBUS_PATH_LEN, "%s/operator/%s%s",
 			modem->path, oper->mcc, oper->mnc);
 
 	return path;
@@ -427,16 +417,16 @@ static DBusMessage *network_operator_get_properties(DBusConnection *conn,
 
 	dbus_gsm_dict_append(&dict, "Status", DBUS_TYPE_STRING, &status);
 
-	if (op->operator->mcc != -1) {
-		dbus_uint16_t mcc = op->operator->mcc;
+	if (*op->operator->mcc != '\0') {
+		const char *mcc = op->operator->mcc;
 		dbus_gsm_dict_append(&dict, "MobileCountryCode",
-					DBUS_TYPE_UINT16, &mcc);
+					DBUS_TYPE_STRING, &mcc);
 	}
 
-	if (op->operator->mnc != -1) {
-		dbus_uint16_t mnc = op->operator->mnc;
+	if (*op->operator->mnc != '\0') {
+		const char *mnc = op->operator->mnc;
 		dbus_gsm_dict_append(&dict, "MobileNetworkCode",
-					DBUS_TYPE_UINT16, &mnc);
+					DBUS_TYPE_STRING, &mnc);
 	}
 
 	if (op->operator->tech != -1) {
-- 
1.6.0.4


^ permalink raw reply related	[flat|nested] 26+ messages in thread

* Re: MNC/MCC as string?
  2009-06-12 10:12                         ` Marcel Holtmann
  2009-06-12 10:53                           ` Aki Niemi
@ 2009-06-12 10:56                           ` =?unknown-8bit?q?R=C3=A9mi?= Denis-Courmont
  1 sibling, 0 replies; 26+ messages in thread
From: =?unknown-8bit?q?R=C3=A9mi?= Denis-Courmont @ 2009-06-12 10:56 UTC (permalink / raw)
  To: ofono

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




On Fri, 12 Jun 2009 12:12:33 +0200, Marcel Holtmann <marcel@holtmann.org>
wrote:
> Hi Aki,
> 
>> Anyway, here it is. Please test; seems to work for me on my N95.
> 
> +static void extract_mcc_mnc(const char *str, char *mcc, char *mnc)
>  {
> -       int num = 0;
> -       unsigned int i;
> -
>         /* Three digit country code */
> -       for (i = 0; i < 3; i++)
> -               num = num * 10 + (int)(str[i] - '0');
> -
> -       *mcc = num;
> -
> -       num = 0;
> +       strncpy(mcc, str, sizeof(mcc));
> +       mcc[3] = '\0';
> 
> maybe I am blind, but how is this suppose to work. The sizeof(mmc) is 1
> byte.

Actually, it is the size of a pointer, which will work (by accident) on
32-bits platform, and overflow on others. 

-- 
Rémi Denis-Courmont


^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: MNC/MCC as string?
  2009-06-12 10:53                           ` Aki Niemi
@ 2009-06-13  1:01                             ` Denis Kenzior
  0 siblings, 0 replies; 26+ messages in thread
From: Denis Kenzior @ 2009-06-13  1:01 UTC (permalink / raw)
  To: ofono

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

Hi Aki,

> Take two attached.

I've pushed the patch out as well as an addendum patch that took care of some 
new and existing style issues.  Also a small fix to the test case as a result 
of the API changes.

Regards,
-Denis


^ permalink raw reply	[flat|nested] 26+ messages in thread

end of thread, other threads:[~2009-06-13  1:01 UTC | newest]

Thread overview: 26+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2009-06-10 11:26 MNC/MCC as string? Aki Niemi
2009-06-10 16:15 ` Denis Kenzior
2009-06-10 16:42   ` =?unknown-8bit?q?R=C3=A9mi?= Denis-Courmont
2009-06-10 17:41     ` Denis Kenzior
2009-06-11 11:46       ` Marcel Holtmann
2009-06-11 14:04         ` Jan Luebbe
2009-06-11 15:09           ` Marcel Holtmann
2009-06-11 14:32         ` Denis Kenzior
2009-06-11 15:12           ` Marcel Holtmann
2009-06-11 18:00           ` Aki Niemi
2009-06-11 18:10             ` Marcel Holtmann
2009-06-11 19:02               ` Aki Niemi
2009-06-11 19:18                 ` Marcel Holtmann
2009-06-11 19:38                   ` Aki Niemi
2009-06-11 20:08                     ` Denis Kenzior
2009-06-12 10:01                       ` Aki Niemi
2009-06-12 10:12                         ` Marcel Holtmann
2009-06-12 10:53                           ` Aki Niemi
2009-06-13  1:01                             ` Denis Kenzior
2009-06-12 10:56                           ` =?unknown-8bit?q?R=C3=A9mi?= Denis-Courmont
2009-06-11 19:58                   ` =?unknown-8bit?q?R=C3=A9mi?= Denis-Courmont
2009-06-11 20:33                     ` Marcel Holtmann
2009-06-11 20:35                 ` Marcel Holtmann
2009-06-11 18:24             ` Denis Kenzior
2009-06-11 18:10         ` Aki Niemi
2009-06-11  6:16   ` Aki Niemi

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.