Re: [PATCH 04/18] gisi: fix subscription for wgmodem2.5

2011-02-16 Thread Aki Niemi
Hi Andreas,

2011/2/15 Andreas Westin andreas.wes...@stericsson.com:
                if (legacy)
                        msg[3 + count] = mux-resource;
 -               else
 +               else {
                        /* Resource field is 32bit and Little-endian */
                        msg[4 + count * 4 + 3] = mux-resource;
 +               }

Curly brackets are either in both if and else, or in neither. I
actually prefer not to have them here, since the first line is just a
comment in the else statement. ;)

                count++;
        }

 +       commgr.spn_dev = legacy ? modem-device : PN_DEV_MODEM;
        len = legacy ? 3 + count : 4 + count * 4;
        msg[2] = count;

This is not necessary, as a modem plugin is supposed to call
g_isi_modem_set_device() to set the modem-device to whatever it needs
to right after creating the GIsiModem instance.

In other words, the assumption is that when the GIsiModem instace is
actually used, all necessary internal data, such as flags and the
remote device have already been set.

Cheers,
Aki
___
ofono mailing list
ofono@ofono.org
http://lists.ofono.org/listinfo/ofono


Re: [PATCH 04/18] gisi: fix subscription for wgmodem2.5

2011-02-16 Thread Andreas WESTIN

Hi Aki,

On 2011-02-16 12:16, Aki Niemi wrote:

Curly brackets are either in both if and else, or in neither. I
actually prefer not to have them here, since the first line is just a
comment in the else statement. ;)


Yes you're right :)


This is not necessary, as a modem plugin is supposed to call
g_isi_modem_set_device() to set the modem-device to whatever it needs
to right after creating the GIsiModem instance.

In other words, the assumption is that when the GIsiModem instace is
actually used, all necessary internal data, such as flags and the
remote device have already been set.


Tested and works fine, will submit a new patch for the u8500 plugin. 
This is not done in the n900 plugin though, perhaps not necessary ?


Regards
Andreas

___
ofono mailing list
ofono@ofono.org
http://lists.ofono.org/listinfo/ofono


Re: [PATCH 04/18] gisi: fix subscription for wgmodem2.5

2011-02-16 Thread Aki Niemi
Hi Andreas,

2011/2/16 Andreas WESTIN andreas.wes...@stericsson.com:
 Tested and works fine, will submit a new patch for the u8500 plugin. This is
 not done in the n900 plugin though, perhaps not necessary ?

The default is PN_DEV_HOST, which is what the N900 uses.

Cheers,
Aki
___
ofono mailing list
ofono@ofono.org
http://lists.ofono.org/listinfo/ofono