Hi Jessica,
2010/12/8 Jessica Nilsson <jessica.j.nils...@stericsson.com>:
> +static void reset_buf(char *buf, char *buf_2, int buf_len)
> +{
> + memset(buf, '\0', buf_len);
> + memset(buf_2, '\0', buf_len);
> +}
> +
> +static int get_topics_len(const char *topics)
> +{
> + int i = 0;
> + int k = 0;
> + int length = 0;
> + char buf[6];
> + char buf_2[6];
> +
> + reset_buf(buf, buf_2, 6);
> +
> + while (*topics != '\0') {
> + if (*topics == ',') {
> + reset_buf(buf, buf_2, 6);
> + k = 0;
> + length++;
> + } else if (*topics != ',' && *topics != '-') {
> + buf[k] = *topics;
> + k++;
> + } else if (*topics == '-') {
> + topics++;
> + i++;
> + k = 0;
> +
> + while (*topics != ',' && *topics != '\0') {
> + buf_2[k] = *topics;
> + topics++;
> + i++;
> + k++;
> + }
> +
> + length = length + atoi(buf_2) - atoi(buf) + 1;
> + k = 0;
> + }
> +
> + topics++;
> + i++;
> + }
> +
> + topics = topics - i;
> + return length;
> +}
> +
> +static void parse_topics(const char *topics, gint16 *topics_parsed)
> +{
> + int j = 0;
> + int k = 0;
> + char buf[6];
> + char buf_2[6];
> +
> + reset_buf(buf, buf_2, 6);
> +
> + while (*topics != '\0') {
> + if (*topics != ',' && *topics != '-') {
> + buf[j] = *topics;
> + j++;
> + } else if (*topics == '-') {
> + topics++;
> + j = 0;
> +
> + while (*topics != ',' && *topics != '\0') {
> + buf_2[j] = *topics;
> + topics++;
> + j++;
> + }
> +
> + for (j = 0; j <= (atoi(buf_2) - atoi(buf)); j++) {
> + topics_parsed[k] = atoi(buf) + j;
> + topics_parsed[k] = g_ntohs(topics_parsed[k]);
> + k++;
> + }
> +
> + j = 0;
> + } else if (*topics == ',') {
> + topics_parsed[k] = atoi(buf);
> + topics_parsed[k] = g_ntohs(topics_parsed[k]);
> + reset_buf(buf, buf_2, 6);
> + j = 0;
> + k++;
> + }
> +
> + topics++;
> + }
> +}
> +
> +static gboolean set_resp_cb(GIsiClient *client, const void *restrict data,
> + size_t len, uint16_t object, void *opaque)
> +{
> + const unsigned char *msg = data;
> + struct isi_cb_data *cbd = opaque;
> + ofono_cbs_set_cb_t cb = cbd->cb;
> +
> + DBG("");
> +
> + if (!msg) {
> + DBG("ISI client error: %d", g_isi_client_error(client));
> + goto error;
> + }
> +
> + if (len < 3 || msg[0] != SMS_CB_ROUTING_RESP)
> + goto error;
> +
> + if (msg[2] != SMS_OK)
> + goto error;
> +
> + cbs_subscription_nr = msg[1];
> + CALLBACK_WITH_SUCCESS(cb, cbd->data);
> + goto out;
> +error:
> + CALLBACK_WITH_FAILURE(cb, cbd->data);
> +out:
> + g_free(cbd);
> + return TRUE;
> +}
> +
> +static void isi_set_topics(struct ofono_cbs *cbs, const char *topics,
> + ofono_cbs_set_cb_t cb, void *data)
> +{
> + struct cbs_data *cd = ofono_cbs_get_data(cbs);
> + struct isi_cb_data *cbd = isi_cb_data_new(cbs, cb, data);
> + int topics_len = get_topics_len(topics);
> + gint16 topics_out[topics_len];
> +
> + uint8_t msg[] = {
> + SMS_CB_ROUTING_REQ,
> + SMS_ROUTING_SET,
> + cbs_subscription_nr,
> + 0x00, /*Subscription type*/
> + 0x00, /*Fillers*/
> + 0x00,
> + 0x01, /*Number of subblocks*/
> + 0x00,
> + 0x26, /*Subblock*/
> + 0x00,
> + topics_len * 2 + 6, /*Subblock length*/
> + 0x00,
> + topics_len, /*Number of topics*/
> + };
> + struct iovec iov[2] = {
> + { msg, sizeof(msg) },
> + { topics_out, sizeof(topics_out) },
> + };
> + parse_topics(topics, topics_out);
> +
> + DBG("");
> +
> + if (g_isi_request_vmake(cd->client, iov, 2, CBS_TIMEOUT,
> + set_resp_cb, cbd))
> + return;
> +
> + CALLBACK_WITH_FAILURE(cb, data);
> + g_free(cbd);
> +}
The CBS topics set is simply not implemented in the current cbs
driver. I suggest you create a patch for adding that into the current
driver.
> +static gboolean clear_resp_cb(GIsiClient *client, const void *restrict data,
> + size_t len, uint16_t object, void *opaque)
> +{
> + const unsigned char *msg = data;
> + struct isi_cb_data *cbd = opaque;
> + ofono_cbs_set_cb_t cb = cbd->cb;
> + DBG("");
> +
> + if (!msg)
> + DBG("ISI client error: %d", g_isi_client_error(client));
> +
> + cbs_subscription_nr = 0;
> + CALLBACK_WITH_SUCCESS(cb, cbd->data);
> + goto out;
> +out:
> + g_free(cbd);
> + return TRUE;
> +}
> +
> +static void isi_clear_topics(struct ofono_cbs *cbs,
> + ofono_cbs_set_cb_t cb, void *data)
> +{
> + struct cbs_data *cd = ofono_cbs_get_data(cbs);
> + struct isi_cb_data *cbd = isi_cb_data_new(cbs, cb, data);
> + uint8_t msg[] = {
> + SMS_CB_ROUTING_REQ,
> + SMS_ROUTING_RELEASE,
> + cbs_subscription_nr, /* Subscription number */
> + 0x00, /* Subscription type */
> + 0x00, /* Fillers */
> + 0x00,
> + 0x00 /*No subblocks*/
> + };
> +
> + DBG("");
> +
> + if (g_isi_request_make(cd->client, msg, sizeof(msg), CBS_TIMEOUT,
> + clear_resp_cb, cbd))
> + return;
> +
> + CALLBACK_WITH_FAILURE(cb, data);
> + g_free(cbd);
> +}
And same for the clear_topics callback.
> diff --git a/drivers/isimodem2.5/checkpatch.pl
> b/drivers/isimodem2.5/checkpatch.pl
> new file mode 100755
> index 0000000..a4d7434
This has no business being in the isimodem driver. I suppose it was
included by mistake.
> +static void isi_query_serial(struct ofono_devinfo *info,
> + ofono_devinfo_query_cb_t cb,
> + void *data)
> +{
> + char imei[16]; /* IMEI 15 digits + 1 null*/
> + char numbers[] = "1234567890";
> + FILE *fp = fopen("/etc/imei", "r");
> + DBG("");
> +
> + if (fp == NULL) {
> + DBG("failed to open /etc/imei file");
> + CALLBACK_WITH_FAILURE(cb, "", data);
> + return;
> + }
> +
> + if (fgets(imei, 16, fp)) {
> + DBG(" IMEI = %s", imei);
> +
> + if (15 == strspn(imei, numbers))
> + CALLBACK_WITH_SUCCESS(cb, imei, data);
> + else
> + CALLBACK_WITH_FAILURE(cb, "", data);
> + }
> +
> + fclose(fp);
> +}
So the IMEI is not stored on the modem at all. This obviously needs a
quirk, but I wonder if it would be cleaner to handle this in the modem
plugin. See for example how GPIO lines are handled in the n900 driver.
> diff --git a/drivers/isimodem2.5/gpds-context.c
> b/drivers/isimodem2.5/gpds-context.c
> new file mode 100644
> index 0000000..5bc9d17
Is there actually anything different here, except for the name change?
It's using a forked version of GIsiPipe, but I'm pretty sure we can
just add the missing bits into the existing GIsiPipe class.
Anyway, the gprs-context driver has changed quite drastically
recently, as the oFono core GPRS support for multiple contexts changed
the way the drivers are allocated.
> +static gboolean decode_reg_status(struct netreg_data *nd, const guint8 *msg,
> + size_t len, int *status, int *lac,
> + int *ci, int *tech)
> +{
> + GIsiSubBlockIter iter;
> + g_isi_sb_iter_init(&iter, msg, len, 0);
> +
> + while (g_isi_sb_iter_is_valid(&iter)) {
> + DBG("Sub-block %s",
> + net_subblock_name(g_isi_sb_iter_get_id(&iter)));
> +
> + switch (g_isi_sb_iter_get_id(&iter)) {
> + case NET_MODEM_REG_INFO_COMMON: {
> + guint8 byte = 0;
> +
> + if (!g_isi_sb_iter_get_byte(&iter, &byte, 2))
> + return FALSE;
> +
> + if (!g_isi_sb_iter_get_byte(&iter,
> + &nd->last_reg_mode,
> 3))
> + return FALSE;
> +
> + *status = byte;
> + break;
> + }
> + case NET_MODEM_GSM_REG_INFO: {
> + guint16 word = 0;
> + guint32 dword = 0;
> + guint8 egprs = 0;
> + guint8 hsdpa = 0;
> + guint8 hsupa = 0;
> +
> + if (!g_isi_sb_iter_get_word(&iter, &word, 2) ||
> + !g_isi_sb_iter_get_dword(&iter, &dword, 4) ||
> + !g_isi_sb_iter_get_byte(&iter, &egprs, 17) ||
> + !g_isi_sb_iter_get_byte(&iter, &hsdpa, 20) ||
> + !g_isi_sb_iter_get_byte(&iter, &hsupa, 21))
> + return FALSE;
> +
> + *ci = (int)dword;
> + *lac = (int)word;
> +
> + switch (nd->rat) {
> + case NET_GSM_RAT:
> + *tech = ACCESS_TECHNOLOGY_GSM;
> +
> + if (nd->gsm_compact)
> + *tech = ACCESS_TECHNOLOGY_GSM_COMPACT;
> + else if (egprs)
> + *tech = ACCESS_TECHNOLOGY_GSM_EGPRS;
> +
> + break;
> + case NET_UMTS_RAT:
> + *tech = 2;
> +
> + if (hsdpa)
> + *tech = ACCESS_TECHNOLOGY_UTRAN_HSDPA;
> +
> + if (hsupa)
> + *tech = ACCESS_TECHNOLOGY_UTRAN_HSUPA;
> +
> + if (hsdpa && hsupa)
> + *tech =
> + ACCESS_TECHNOLOGY_UTRAN_HSDPA_HSUPA;
> +
> + break;
> + default:
> + *tech = ACCESS_TECHNOLOGY_GSM;
> + }
> +
> + break;
> + }
> + default:
> + DBG("Skipping sub-block: %s (%zu bytes)",
> +
> net_subblock_name(g_isi_sb_iter_get_id(&iter)),
> + g_isi_sb_iter_get_len(&iter));
> + break;
> + }
> +
> + g_isi_sb_iter_next(&iter);
> + }
> +
> + DBG("status=%d, lac=%d, ci=%d, tech=%d",
> + (*status), *lac, *ci, *tech);
> + return TRUE;
> +}
Just adding the decoding for NET_MODEM_ subblocks here would be
enough. In fact, these subblocks look almost identical to the existing
NET_ subblocks, so adding just extra case statements with the new
codepoints would probably work, too.
> +static void cs_power_on_req(GIsiClient *client)
> +{
> + const unsigned char msg[] = {
> + NET_CS_CONTROL_REQ,
> + 0x03,
> + 0x00, /* Sub-block count */
> + };
> + DBG("");
> +
> + /* Just send message, ignore the reponse */
> + g_isi_request_make(client, msg, sizeof(msg),
> + NETWORK_SET_TIMEOUT,
> + NULL , NULL);
> +}
I think this is a bad idea. I know you're using NET_CS_CONTROL_REQ
here to implement the unregister callback, but the this is basically
just wrapping existing PN_MTC functionality. In general, ISI modems
don't seem to understand what a deregister even means.
In fact, even the D-Bus method seems of little value, and I would
remove it altogether. In any case, leaving this unimplemented is my
preference.
> + /* TODO Here we should notify framework once it
> + * supports this indication e.g.
> + * ofono_netreg_name_notify(netreg, op.name);
> + */
Right, we need to figure out how NITZ names are to be supported,
particularly with ISI modems.
I've sent some TODO items for this, but we need to figure out how this
is going to work with AT modems as well.
> +static void isi_strength(struct ofono_netreg *netreg,
> + ofono_netreg_strength_cb_t cb,
> + void *data)
> +{
> + struct netreg_data *nd = ofono_netreg_get_data(netreg);
> + struct isi_cb_data *cbd = isi_cb_data_new(netreg, cb, data);
> + const unsigned char msg[] = {
> + NET_RSSI_GET_REQ,
> + NET_CS_GSM,
> + NET_CURRENT_CELL_RSSI,
> + 0, 0, 0, 0 /* Week 18 specs requires these */
This is different from the current netreg driver. However, it's easy
enough to alternate, see below.
> +static void reachable_cb(GIsiClient *client, gboolean alive, uint16_t object,
> + void *opaque)
> +{
> + struct ofono_netreg *netreg = opaque;
> +
> + if (!alive) {
> + DBG("Unable to bootsrap netreg driver");
> + return;
> + }
> +
> + DBG("Resource %s (v%03d.%03d) reachable",
> + pn_resource_name(g_isi_client_resource(client)),
> + g_isi_version_major(client),
> + g_isi_version_minor(client));
> + g_idle_add(isi_netreg_register, netreg);
> +}
> +
> +static int isi_netreg_probe(struct ofono_netreg *netreg, unsigned int vendor,
> + void *user)
> +{
> + GIsiModem *idx = user;
> + struct netreg_data *nd = g_try_new0(struct netreg_data, 1);
> +
> + if (!nd)
> + return -ENOMEM;
> +
> + nd->client = g_isi_client_create(idx, PN_MODEM_NETWORK);
> +
> + if (!nd->client) {
> + g_free(nd);
> + return -ENOMEM;
> + }
> +
> + ofono_netreg_set_data(netreg, nd);
> + g_isi_verify(nd->client, reachable_cb, netreg);
> + nd->cs_active = FALSE; /* Assume CS is inactive (updated later) */
> + return 0;
> +}
I would suggest probing for both PN_NETWORK and PN_MODEM_NETWORK here.
Then which ever answers first gets used. After we know with which
resource we're working with, overloading a few of the ISI messages is
enough to make this driver work with both versions of modems.
I didn't go through all of the patch, but from what I can see, we can
pretty well fold the WG2.5 modem support into the existing drivers.
Cheers,
Aki
_______________________________________________
ofono mailing list
ofono@ofono.org
http://lists.ofono.org/listinfo/ofono
