[PATCH 1/1] gisi: Updated subscriptions and pipe handling to accomodate additional isimodem versions
---
This is needed in order for isimodem2.5 to work.
Best Regards,
Jessica
gisi/common.h |3 +
gisi/message.c |8 +
gisi/message.h |1 +
gisi/modem.c | 105 --
gisi/modem.h |4 +
gisi/pipe.c| 433 ++--
6 files changed, 531 insertions(+), 23 deletions(-)
diff --git a/gisi/common.h b/gisi/common.h
index 83a8cf5..c78f893 100644
--- a/gisi/common.h
+++ b/gisi/common.h
@@ -26,6 +26,8 @@
extern "C" {
#endif
+#define PN_HOST0x00
+#define PN_MODEM 0x60
#define PN_COMMGR 0x10
#define PN_NAMESERVICE 0xDB
#define PN_FIREWALL0x43
@@ -35,6 +37,7 @@ enum message_id {
PNS_NAME_ADD_REQ = 0x05,
PNS_NAME_REMOVE_REQ = 0x07,
PNS_SUBSCRIBED_RESOURCES_IND = 0x10,
+ PNS_SUBSCRIBED_RESOURCES_EXTEND_IND = 0x12,
COMM_ISI_VERSION_GET_REQ = 0x12,
COMM_ISI_VERSION_GET_RESP = 0x13,
COMM_ISA_ENTITY_NOT_REACHABLE_RESP =0x14,
diff --git a/gisi/message.c b/gisi/message.c
index 8f4fe5a..9d00108 100644
--- a/gisi/message.c
+++ b/gisi/message.c
@@ -65,6 +65,14 @@ uint8_t g_isi_msg_resource(const GIsiMessage *msg)
return msg->addr->spn_resource;
}
+uint8_t g_isi_msg_device(const GIsiMessage *msg)
+{
+ if (msg == NULL || msg->addr == NULL)
+ return 0;
+
+ return msg->addr->spn_dev;
+}
+
uint16_t g_isi_msg_object(const GIsiMessage *msg)
{
if (msg == NULL || msg->addr == NULL)
diff --git a/gisi/message.h b/gisi/message.h
index 95348f8..f34ce57 100644
--- a/gisi/message.h
+++ b/gisi/message.h
@@ -52,6 +52,7 @@ int g_isi_msg_version_minor(const GIsiMessage *msg);
int g_isi_msg_error(const GIsiMessage *msg);
const char *g_isi_msg_strerror(const GIsiMessage *msg);
uint8_t g_isi_msg_resource(const GIsiMessage *msg);
+uint8_t g_isi_msg_device(const GIsiMessage *msg);
uint16_t g_isi_msg_object(const GIsiMessage *msg);
uint8_t g_isi_msg_id(const GIsiMessage *msg);
diff --git a/gisi/modem.c b/gisi/modem.c
index 8750367..7201235 100644
--- a/gisi/modem.c
+++ b/gisi/modem.c
@@ -61,6 +61,7 @@ struct _GIsiModem {
unsigned index;
GHashTable *services;
gboolean subs_source;
+ GIsiVersion version;
int req_fd;
int ind_fd;
guint req_watch;
@@ -87,11 +88,6 @@ static const struct sockaddr_pn namesrv = {
.spn_resource = PN_NAMESERVICE,
};
-static const struct sockaddr_pn commgr = {
- .spn_family = AF_PHONET,
- .spn_resource = PN_COMMGR,
-};
-
static GIsiServiceMux *service_get(GIsiModem *modem, uint8_t resource)
{
GIsiServiceMux *mux;
@@ -349,11 +345,34 @@ static gboolean modem_subs_update(gpointer data)
gpointer keyptr, value;
GIsiModem *modem = data;
- uint8_t msg[3 + 256] = {
- 0, PNS_SUBSCRIBED_RESOURCES_IND,
- 0,
- };
+ GIsiMessage dmsg;
+ uint8_t msg[3 + 256] = { 0 };
uint8_t count = 0;
+ int msg_size = 0;
+ struct sockaddr_pn commgr = {
+ .spn_family = AF_PHONET,
+ .spn_resource = PN_COMMGR,
+ };
+
+ if (g_isi_modem_version_major(modem) == 2 &&
+ g_isi_modem_version_minor(modem) == 5) {
+ uint8_t s_msg[4] = {
+ 0, PNS_SUBSCRIBED_RESOURCES_EXTEND_IND,
+ 0,
+ 0, /* filler */
+ };
+ g_memmove(msg, s_msg, 4);
+ msg_size = 4;
+
+ commgr.spn_dev = PN_MODEM;
+ } else {
+ uint8_t s_msg[3] = {
+ 0, PNS_SUBSCRIBED_RESOURCES_IND,
+ 0,
+ };
+ g_memmove(msg, s_msg, 3);
+ msg_size = 3;
+ }
modem->subs_source = 0;
@@ -363,13 +382,31 @@ static gboolean modem_subs_update(gpointer data)
GIsiServiceMux *mux = value;
if (mux->subscriptions > 0) {
- msg[3 + count] = mux->resource;
+
count++;
+
+ if (g_isi_modem_version_major(modem) == 2 &&
+ g_isi_modem_version_minor(modem) == 5) {
+ msg[3 + (count * 4)] = mux->resource;
+ msg_size += 4;
+ } else {
+ msg[2 + count] = mux->resource;
+ msg_size += count;
+ }
+
}
}
msg[2] = count;
- sendto(modem->ind_fd, msg, 3 + msg[2], MSG_NOSIGNAL, (void *)&commgr,
+ dmsg.addr = &commgr;
+ dmsg.error = 0;
+ dmsg.data = msg;
+ dmsg.len = msg_size;
+
+ if (modem->trace != NULL)
Re: [PATCH 1/1] gisi: Updated subscriptions and pipe handling to accomodate additional isimodem versions
Hi, 2011/2/4 Aki Niemi : > This is problematic, since there is no version query by default for > the PN_COMMGR because creating the GIsiModem is not async. Also, even > the N900 modem supports both of these INDs, but the > PNS_SUBSCRIBED_RESOURCES_EXTEND_IND is blocked by a modem side > firewall. > > So I think we need something different here, like flags that the > plugin can set to control which IND type gets used. I just pushed a new API to set flags on a modem instance, and also refactored the indication subscription logic based on this patch. Please check that it works like you want it to. Cheers, Aki ___ ofono mailing list ofono@ofono.org http://lists.ofono.org/listinfo/ofono
Re: [PATCH 1/1] gisi: Updated subscriptions and pipe handling to accomodate additional isimodem versions
Hi Rémi,
2011/1/28 Rémi Denis-Courmont :
> On Friday 28 January 2011 13:37:10 ext Jessica Nilsson, you wrote:
>> diff --git a/gisi/common.h b/gisi/common.h
>> index 83a8cf5..c78f893 100644
>> --- a/gisi/common.h
>> +++ b/gisi/common.h
>> @@ -26,6 +26,8 @@
>> extern "C" {
>> #endif
>>
>> +#define PN_HOST 0x00
>> +#define PN_MODEM 0x60
>
> It looks like you are mixing up services (PN_* from 0 to 255), with device
> addresses (PN_DEV_* multiple of four from 0 to 252).
>
>> #define PN_COMMGR 0x10
>> #define PN_NAMESERVICE 0xDB
>> #define PN_FIREWALL 0x43
>> diff --git a/gisi/modem.c b/gisi/modem.c
>> index 8750367..7201235 100644
>> --- a/gisi/modem.c
>> +++ b/gisi/modem.c
>> @@ -61,6 +61,7 @@ struct _GIsiModem {
>> unsigned index;
>> GHashTable *services;
>> gboolean subs_source;
>> + GIsiVersion version;
>
> I can't see where this gets set?!
>
>> int req_fd;
>> int ind_fd;
>> guint req_watch;
>> @@ -349,11 +345,34 @@ static gboolean modem_subs_update(gpointer data)
>> gpointer keyptr, value;
>>
>> GIsiModem *modem = data;
>> - uint8_t msg[3 + 256] = {
>> - 0, PNS_SUBSCRIBED_RESOURCES_IND,
>> - 0,
>> - };
>> + GIsiMessage dmsg;
>> + uint8_t msg[3 + 256] = { 0 };
>> uint8_t count = 0;
>> + int msg_size = 0;
>> + struct sockaddr_pn commgr = {
>> + .spn_family = AF_PHONET,
>> + .spn_resource = PN_COMMGR,
>> + };
>> +
>> + if (g_isi_modem_version_major(modem) == 2 &&
>> + g_isi_modem_version_minor(modem) == 5) {
>
> You really need to factor this check out. It's going to be a disaster when
> there are more than two versions otherwise.
This is problematic, since there is no version query by default for
the PN_COMMGR because creating the GIsiModem is not async. Also, even
the N900 modem supports both of these INDs, but the
PNS_SUBSCRIBED_RESOURCES_EXTEND_IND is blocked by a modem side
firewall.
So I think we need something different here, like flags that the
plugin can set to control which IND type gets used.
Cheers,
Aki
___
ofono mailing list
ofono@ofono.org
http://lists.ofono.org/listinfo/ofono
Re: [PATCH 1/1] gisi: Updated subscriptions and pipe handling to accomodate additional isimodem versions
Hi Rémi,
2011/1/28 Rémi Denis-Courmont
>
> It looks like you are mixing up services (PN_* from 0 to 255), with device
> addresses (PN_DEV_* multiple of four from 0 to 252).
Yes, you are right. We will change it.
>
> > @@ -61,6 +61,7 @@ struct _GIsiModem {
> > unsigned index;
> > GHashTable *services;
> > gboolean subs_source;
> > + GIsiVersion version;
>
> I can't see where this gets set?!
In g_isi_modem_set_version() in plugins/u8500.c
New file, that will hopefully be sent in a patch to the mailing list today.
>
> > +
> > + if (g_isi_modem_version_major(modem) == 2 &&
> > + g_isi_modem_version_minor(modem) == 5) {
>
> You really need to factor this check out. It's going to be a disaster when
> there are more than two versions otherwise.
Well, we really need the PNS_SUBSCRIBED_RESOURCES_EXTEND_IND because
the other one doesn't work for us.
Would a lookup function do the trick, with filling in the necessary
values in msg depending on which version it is?
If that is not satisfactory, could you please be a bit more specific
in what you would like to see so we don't misunderstand?
>
> >
> > @@ -363,13 +382,31 @@ static gboolean modem_subs_update(gpointer data)
>
>
> That should probably be a separate patch.
Yes, you are right.
>
> > diff --git a/gisi/pipe.c b/gisi/pipe.c
>
>
> We already have this in kernel and I wonder why this needs to be duplicated in
> userspace now?! Furthermore pipe.c is about pipes. Pipe endpoints code belongs
> in pep.c. If you want to implement a pipe controller in gisi, then I think it
> really deserves a new file of its own.
>
isimodem2.5 needs this code, the kernel struct won't be applicable for
us in the modem case.
We will move the pep functions to the pep.c
Best Regards,
Jessica Nilsson
ST-Ericsson
___
ofono mailing list
ofono@ofono.org
http://lists.ofono.org/listinfo/ofono
Re: [PATCH 1/1] gisi: Updated subscriptions and pipe handling to accomodate additional isimodem versions
On Friday 28 January 2011 13:37:10 ext Jessica Nilsson, you wrote:
> diff --git a/gisi/common.h b/gisi/common.h
> index 83a8cf5..c78f893 100644
> --- a/gisi/common.h
> +++ b/gisi/common.h
> @@ -26,6 +26,8 @@
> extern "C" {
> #endif
>
> +#define PN_HOST0x00
> +#define PN_MODEM 0x60
It looks like you are mixing up services (PN_* from 0 to 255), with device
addresses (PN_DEV_* multiple of four from 0 to 252).
> #define PN_COMMGR 0x10
> #define PN_NAMESERVICE 0xDB
> #define PN_FIREWALL0x43
> diff --git a/gisi/modem.c b/gisi/modem.c
> index 8750367..7201235 100644
> --- a/gisi/modem.c
> +++ b/gisi/modem.c
> @@ -61,6 +61,7 @@ struct _GIsiModem {
> unsigned index;
> GHashTable *services;
> gboolean subs_source;
> + GIsiVersion version;
I can't see where this gets set?!
> int req_fd;
> int ind_fd;
> guint req_watch;
> @@ -349,11 +345,34 @@ static gboolean modem_subs_update(gpointer data)
> gpointer keyptr, value;
>
> GIsiModem *modem = data;
> - uint8_t msg[3 + 256] = {
> - 0, PNS_SUBSCRIBED_RESOURCES_IND,
> - 0,
> - };
> + GIsiMessage dmsg;
> + uint8_t msg[3 + 256] = { 0 };
> uint8_t count = 0;
> + int msg_size = 0;
> + struct sockaddr_pn commgr = {
> + .spn_family = AF_PHONET,
> + .spn_resource = PN_COMMGR,
> + };
> +
> + if (g_isi_modem_version_major(modem) == 2 &&
> + g_isi_modem_version_minor(modem) == 5) {
You really need to factor this check out. It's going to be a disaster when
there are more than two versions otherwise.
> + uint8_t s_msg[4] = {
> + 0, PNS_SUBSCRIBED_RESOURCES_EXTEND_IND,
> + 0,
> + 0, /* filler */
> + };
> + g_memmove(msg, s_msg, 4);
> + msg_size = 4;
> +
> + commgr.spn_dev = PN_MODEM;
> + } else {
> + uint8_t s_msg[3] = {
> + 0, PNS_SUBSCRIBED_RESOURCES_IND,
> + 0,
> + };
> + g_memmove(msg, s_msg, 3);
> + msg_size = 3;
> + }
>
> modem->subs_source = 0;
>
> @@ -363,13 +382,31 @@ static gboolean modem_subs_update(gpointer data)
> GIsiServiceMux *mux = value;
>
> if (mux->subscriptions > 0) {
> - msg[3 + count] = mux->resource;
> +
> count++;
> +
> + if (g_isi_modem_version_major(modem) == 2 &&
> + g_isi_modem_version_minor(modem) == 5) {
> + msg[3 + (count * 4)] = mux->resource;
> + msg_size += 4;
> + } else {
> + msg[2 + count] = mux->resource;
> + msg_size += count;
> + }
> +
> }
> }
> msg[2] = count;
>
> - sendto(modem->ind_fd, msg, 3 + msg[2], MSG_NOSIGNAL, (void
> *)&commgr, + dmsg.addr = &commgr;
> + dmsg.error = 0;
> + dmsg.data = msg;
> + dmsg.len = msg_size;
> +
> + if (modem->trace != NULL)
> + modem->trace(&dmsg, NULL);
> +
> + sendto(modem->ind_fd, msg, msg_size, MSG_NOSIGNAL, (void *)&commgr,
> sizeof(commgr));
That should probably be a separate patch.
>
> return FALSE;
> diff --git a/gisi/pipe.c b/gisi/pipe.c
> index 1bd5140..14d8b1c 100644
> --- a/gisi/pipe.c
> +++ b/gisi/pipe.c
> @@ -24,15 +24,22 @@
> #endif
>
> #include
> +#include
> +#include
> +#include
> +#include
> #include
> #include
>
> #include "client.h"
> #include "pipe.h"
> +#include "common.h"
>
> #define PN_PIPE0xD9
> #define PN_PIPE_INVALID_HANDLE 0xFF
>
> +#define PN_OBJ_PEP_GPRS 0x30
> +
> struct isi_pipe_create_req {
> uint8_t cmd;
> uint8_t state_after;
> @@ -73,6 +80,53 @@ struct isi_pipe_resp {
> uint8_t error2;
> };
>
> +struct isi_pep_connect_req{
> + uint8_t cmd;
> + uint8_t pipe_handle;
> + uint8_t state_after;
> + uint8_t other_pep_type;
> + uint8_t filler1;
> + uint8_t filler2;
> + uint8_t n_sb;
> + uint8_t data_sb;
> + uint8_t sb_len;
> + uint8_t alignment;
> + uint8_t filler;
> +};
> +
> +struct isi_pep_enable_req{
> + uint8_t cmd;
> + uint8_t pipe_handle;
> + uint8_t filler;
> +};
> +
> +struct isi_pep_disconnect_req{
> + uint8_t cmd;
> + uint8_t pipe_handle;
> + uint8_t filler;
> +};
> +
> +struct isi_pep_resp {
> + uint8_t pipe_handle;
> + uint8_t error_code;
> +};
> +
> +struct isi_pipe_created_ind{
>
Re: [PATCH 1/1] gisi: Updated subscriptions and pipe handling to accomodate additional isimodem versions
Hi Marcel, On Fri, Jan 28, 2011 at 12:42 PM, Marcel Holtmann wrote: > you might wanna keep the subject line under 50 characters and have a > more detailed explanation in the commit body. See M5. How did I miss that one? I will fix it! > Also you might need a From line at the top. I think the mail server has been creative, because it was there when I sent it. I'll see if I can change the settings somehow, I'll wait for more comments on the actual code before sending an new patch though, is that ok? Best Regards, Jessica Nilsson ST-Ericsson ___ ofono mailing list ofono@ofono.org http://lists.ofono.org/listinfo/ofono
Re: [PATCH 1/1] gisi: Updated subscriptions and pipe handling to accomodate additional isimodem versions
Hi Jessica, > --- > This is needed in order for isimodem2.5 to work. you might wanna keep the subject line under 50 characters and have a more detailed explanation in the commit body. See M5. Also you might need a From line at the top. Regards Marcel ___ ofono mailing list ofono@ofono.org http://lists.ofono.org/listinfo/ofono
[PATCH 1/1] gisi: Updated subscriptions and pipe handling to accomodate additional isimodem versions
---
This is needed in order for isimodem2.5 to work.
Best Regards,
Jessica Nilsson
ST-Ericsson
gisi/common.h |3 +
gisi/message.c |8 +
gisi/message.h |1 +
gisi/modem.c | 105 --
gisi/modem.h |4 +
gisi/pipe.c| 433 ++--
6 files changed, 531 insertions(+), 23 deletions(-)
diff --git a/gisi/common.h b/gisi/common.h
index 83a8cf5..c78f893 100644
--- a/gisi/common.h
+++ b/gisi/common.h
@@ -26,6 +26,8 @@
extern "C" {
#endif
+#define PN_HOST0x00
+#define PN_MODEM 0x60
#define PN_COMMGR 0x10
#define PN_NAMESERVICE 0xDB
#define PN_FIREWALL0x43
@@ -35,6 +37,7 @@ enum message_id {
PNS_NAME_ADD_REQ = 0x05,
PNS_NAME_REMOVE_REQ = 0x07,
PNS_SUBSCRIBED_RESOURCES_IND = 0x10,
+ PNS_SUBSCRIBED_RESOURCES_EXTEND_IND = 0x12,
COMM_ISI_VERSION_GET_REQ = 0x12,
COMM_ISI_VERSION_GET_RESP = 0x13,
COMM_ISA_ENTITY_NOT_REACHABLE_RESP =0x14,
diff --git a/gisi/message.c b/gisi/message.c
index 8f4fe5a..9d00108 100644
--- a/gisi/message.c
+++ b/gisi/message.c
@@ -65,6 +65,14 @@ uint8_t g_isi_msg_resource(const GIsiMessage *msg)
return msg->addr->spn_resource;
}
+uint8_t g_isi_msg_device(const GIsiMessage *msg)
+{
+ if (msg == NULL || msg->addr == NULL)
+ return 0;
+
+ return msg->addr->spn_dev;
+}
+
uint16_t g_isi_msg_object(const GIsiMessage *msg)
{
if (msg == NULL || msg->addr == NULL)
diff --git a/gisi/message.h b/gisi/message.h
index 95348f8..f34ce57 100644
--- a/gisi/message.h
+++ b/gisi/message.h
@@ -52,6 +52,7 @@ int g_isi_msg_version_minor(const GIsiMessage *msg);
int g_isi_msg_error(const GIsiMessage *msg);
const char *g_isi_msg_strerror(const GIsiMessage *msg);
uint8_t g_isi_msg_resource(const GIsiMessage *msg);
+uint8_t g_isi_msg_device(const GIsiMessage *msg);
uint16_t g_isi_msg_object(const GIsiMessage *msg);
uint8_t g_isi_msg_id(const GIsiMessage *msg);
diff --git a/gisi/modem.c b/gisi/modem.c
index 8750367..7201235 100644
--- a/gisi/modem.c
+++ b/gisi/modem.c
@@ -61,6 +61,7 @@ struct _GIsiModem {
unsigned index;
GHashTable *services;
gboolean subs_source;
+ GIsiVersion version;
int req_fd;
int ind_fd;
guint req_watch;
@@ -87,11 +88,6 @@ static const struct sockaddr_pn namesrv = {
.spn_resource = PN_NAMESERVICE,
};
-static const struct sockaddr_pn commgr = {
- .spn_family = AF_PHONET,
- .spn_resource = PN_COMMGR,
-};
-
static GIsiServiceMux *service_get(GIsiModem *modem, uint8_t resource)
{
GIsiServiceMux *mux;
@@ -349,11 +345,34 @@ static gboolean modem_subs_update(gpointer data)
gpointer keyptr, value;
GIsiModem *modem = data;
- uint8_t msg[3 + 256] = {
- 0, PNS_SUBSCRIBED_RESOURCES_IND,
- 0,
- };
+ GIsiMessage dmsg;
+ uint8_t msg[3 + 256] = { 0 };
uint8_t count = 0;
+ int msg_size = 0;
+ struct sockaddr_pn commgr = {
+ .spn_family = AF_PHONET,
+ .spn_resource = PN_COMMGR,
+ };
+
+ if (g_isi_modem_version_major(modem) == 2 &&
+ g_isi_modem_version_minor(modem) == 5) {
+ uint8_t s_msg[4] = {
+ 0, PNS_SUBSCRIBED_RESOURCES_EXTEND_IND,
+ 0,
+ 0, /* filler */
+ };
+ g_memmove(msg, s_msg, 4);
+ msg_size = 4;
+
+ commgr.spn_dev = PN_MODEM;
+ } else {
+ uint8_t s_msg[3] = {
+ 0, PNS_SUBSCRIBED_RESOURCES_IND,
+ 0,
+ };
+ g_memmove(msg, s_msg, 3);
+ msg_size = 3;
+ }
modem->subs_source = 0;
@@ -363,13 +382,31 @@ static gboolean modem_subs_update(gpointer data)
GIsiServiceMux *mux = value;
if (mux->subscriptions > 0) {
- msg[3 + count] = mux->resource;
+
count++;
+
+ if (g_isi_modem_version_major(modem) == 2 &&
+ g_isi_modem_version_minor(modem) == 5) {
+ msg[3 + (count * 4)] = mux->resource;
+ msg_size += 4;
+ } else {
+ msg[2 + count] = mux->resource;
+ msg_size += count;
+ }
+
}
}
msg[2] = count;
- sendto(modem->ind_fd, msg, 3 + msg[2], MSG_NOSIGNAL, (void *)&commgr,
+ dmsg.addr = &commgr;
+ dmsg.error = 0;
+ dmsg.data = msg;
+ dmsg.len = msg_size;
+
+ if (
