Android's roaming detection & its implementation

I know I wrote about Android already today, but there is another thing that concerns me right now. I am owner of an Android-based phone (an HTC Dream) and recently switched my mobile network provider. The problem is that my new provider is a virtual provider and as such there is no real network of that provider. Now Android has a feature to turn off broadband connections when in roaming mode, which itself is a great idea and can save you from paying quite a lot of money when the phone connects to 3G abroad, but this feature also turns off broadband connections when roaming locally. All this is being discussed in bug report #3499.

After noticing this problem I became curious on how Android detects that it is roaming and I found the GsmServiceStateTracker.isRoamingBetweenOperators method to be responsible for that magic, but soon noticed that the method is not only inefficient, but also doesn't work as intended. This is hardly related to the bug mentioned above, but let's have a look at the code in question:

/**
* Set roaming state when gsmRoaming is true and, if operator mcc is the
* same as sim mcc, ons is different from spn
* @param gsmRoaming TS 27.007 7.2 CREG registered roaming
* @param s ServiceState hold current ons
* @return true for roaming state set
*/
    private
    boolean isRoamingBetweenOperators(boolean gsmRoaming, ServiceState s) {
        String spn = SystemProperties.get(PROPERTY_ICC_OPERATOR_ALPHA, "empty");

        String onsl = s.getOperatorAlphaLong();
        String onss = s.getOperatorAlphaShort();

        boolean equalsOnsl = onsl != null && spn.equals(onsl);
        boolean equalsOnss = onss != null && spn.equals(onss);

        String simNumeric = SystemProperties.get(PROPERTY_ICC_OPERATOR_NUMERIC, "");
        String operatorNumeric = s.getOperatorNumeric();

        boolean equalsMcc = true;
        try {
            equalsMcc = simNumeric.substring(0, 3).
                    equals(operatorNumeric.substring(0, 3));
        } catch (Exception e){
        }

        return gsmRoaming && !(equalsMcc && (equalsOnsl || equalsOnss));
    }

Okay, let me summarize what this piece of code does wrong, at least from my understanding:

• It takes both the network operator alphanumeric identifier and alphanumeric long identifier and compares both to the alphanumeric identifier coming from the SIM card, whilst...


• ... it could simply use the network and SIM card numeric identifiers and compare those, which should be a lot cheaper than comparing those strings


• Then it takes the first three characters/digits of the numeric identifiers (which indicate the country) and compares those

Now in my case my SIM card doesn't seem to provide the phone with a alphanumeric identifier, so the first two comparisons always fail for obvious reasons and, looking at the inline-if in the last line of that method my phone will always indicate that I am in roaming mode, even when I am not.

The problem is not only the logic which seems to be wrong, but I rather see the inefficient comparisons used there to be a major problem in embedded systems like mobile phones. This is the first piece of Android code I have had a look at, but if all other code is as ugly and inefficient as these few lines Android really needs some major fixes. Related to this I have reported bug #4590 and forked the git repository in question over at github, to fix this method, should be a matter of 5 minutes.