Hi Iiro,
2011/8/3 <iiro.kaihlaniemi(a)nokia.com>:
diff --git a/drivers/isimodem/network-registration.c
b/drivers/isimodem/network-registration.c
index cc48579..ffcfab4 100644
--- a/drivers/isimodem/network-registration.c
+++ b/drivers/isimodem/network-registration.c
@@ -946,6 +946,84 @@ error:
g_free(cbd);
}
+static void cs_access_config_resp_cb(const GIsiMessage *msg, void *data)
+{
+ struct ofono_netreg *netreg = data;
+ struct netreg_data *nd = ofono_netreg_get_data(netreg);
+ GIsiSubBlockIter iter;
+
+ DBG("");
+ if (g_isi_msg_id(msg) != NET_NW_ACCESS_CONF_RESP)
+ return;
Indent with a single tab, no spaces allowed. This applies to the rest
of the patch as well.
+ for (g_isi_sb_iter_init(&iter, msg, 2);
+ g_isi_sb_iter_is_valid(&iter);
+ g_isi_sb_iter_next(&iter))
+ {
+ uint8_t id = g_isi_sb_iter_get_id(&iter);
+ uint8_t mode;
+
+ DBG("SB=%02X", id);
+ if ((id == 0x55) || (id == 0x59))
+ {
+ g_isi_sb_iter_get_byte(&iter, &mode, 2);
+ DBG("Reg %X", mode);
+ }
+ else if ((id == 0x56) || (id == 0x5A))
+ {
+ g_isi_sb_iter_get_byte(&iter, &mode, 2);
+ DBG("Roam %X", mode);
+ }
These subblock IDs need to be added in network.h. I would also
recommend a switch statement here for more readability.
+ else
+ {
+ DBG("Unknown subblock");
+ }
+ }
Regarding the entire for and if blocks above and elsewhere in the
patch, the style in oFono is to attach curly brackets on the same line
as the for/if/while statement. Please take a look at
doc/coding-style.txt.
To catch style errors early, I would recommend also using the
checkpatch.pl script before submitting.
+}
+
+static void cs_state_resp_cb(const GIsiMessage *msg, void *data)
+{
+ struct ofono_netreg *netreg = data;
+ struct netreg_data *nd = ofono_netreg_get_data(netreg);
+ uint8_t code;
+
+ DBG("");
+ if (g_isi_msg_id(msg) != NET_CS_STATE_RESP)
+ return;
+
+ if (!g_isi_msg_data_get_byte(msg, 0, &code))
+ return;
+
+ if (code != NET_CAUSE_OK)
+ {
+ DBG("Failed with cause=%X", code);
+ return;
+ }
+
+ if (!g_isi_msg_data_get_byte(msg, 1, &code))
+ return;
+ DBG("CS STATE=%X", code);
+
+ if (code == NET_CS_INACTIVE)
+ {
+ DBG("CS INACTIVE - DO POWER ON NOT IMPLEMENTED!!!!!!");
No shouting please ;)
+ }
+ else
+ {
+ /* Enable registration and roaming */
+ const uint8_t req[] = {
+ NET_NW_ACCESS_CONF_REQ, 0, 2,
+ /* Subblock 1 */
+ 0x59, 4, 1, 0,
+ /* Subblock 2 */
+ 0x5A, 4, 1, 0,
Should really add these subblock IDs in network.h.
+ };
+
+ DBG("CS ACTIVE - Check access config");
+ g_isi_client_send(nd->client, req, sizeof(req), cs_access_config_resp_cb,
netreg, NULL);
Please take a look at coding style rule O2, and rather than putting
all this in an else statement, just return above if code ==
NET_CS_INACTIVE.
By the way, is CS access config ever set if when probing the CS state
is NET_CS_INACTIVE? Should we also subscribe to the equivalent
indication to drive this code?
+ }
+}
+
static void subscribe_indications(GIsiClient *cl, void *data)
{
g_isi_client_ind_subscribe(cl, NET_RSSI_IND, rssi_ind_cb, data);
@@ -994,6 +1072,11 @@ static void pn_modem_network_reachable_cb(const GIsiMessage *msg,
void *data)
struct ofono_netreg *netreg = data;
struct netreg_data *nd = ofono_netreg_get_data(netreg);
+ const uint8_t req[] = {
+ NET_CS_STATE_REQ,
+ };
+
+
Indent with tabs here.
Cheers,
Aki