Re: ModemManager: SMS List API doesn't have an index, buffer isn't big enough
On Fri, 2011-04-08 at 18:09 -0400, Nathan Williams wrote:
> OK, the spew-control property is easy enough; see attached. However,
> being new to this codebase, I'm not sure where probing is happening
> and where I should be toggling this property on.
1c3101fbaf74cbb9d8d0eb52ad6f84dc01544cf0
The place you want is in
src/mm-plugin-base.c::mm_plugin_base_probe_port() where the probing
serial port gets opened. Since you defaulted spew control to FALSE, we
want to turn it on in that function. The probe ports will be closed
after probing is complete, and after that ports opened shouldn't have
spew control set at all due to the default.
Dan
>
> - Nathan
>
> On Wed, Apr 6, 2011 at 7:21 PM, Dan Williams wrote:
> On Wed, 2011-04-06 at 12:16 -0400, Nathan Williams wrote:
> > Hi, all. In the process of implementing some SMS support,
> I've run
> > into a couple of issues with the
> > org.freedesktop.ModemManager.Modem.Gsm.SMS.List DBus call,
> one in the
> > API and one in the implementation.
>
>
> Excellent :) We can change the API because for all practical
> purposes,
> it's not being used yet.
>
> > The API issue is that as specified, the caller gets the
> contents of
> > all of the messages, but not their index numbers, so there's
> no way to
> > delete the messages that have been received. Here are three
> ways I've
> > considered fixing this:
> >
> >
> > 1. Change to a List command that just gets the index
> numbers of the
> > messages and lets the caller Get and Delete from that as
> needed. This
> > is an OK API but doesn't really match the AT commands and
> would be a
> > bit inefficient in that regard.
> > 2. Change the return type from "aa{sv}" to something that
> includes
> > the index distinct from the message, such as "a(ia{sv})".
> Reflects the
> > low level well, creates a bit of extra assembly and
> disassembly work.
> > 3. Add an "index" integer element to the dictionary of each
> message.
> > This is both easy and still pretty closely reflects the
> low-level
> > operation.
> >
> >
> > I'm leaning towards #3.
>
>
> Yeah, I agree. #2 maps more closely to the other methods
> since they all
> take top-level "u" arguments, but having it in the dict isn't
> a problem.
> But make sure you make the 'index' item type uint32 ("u") to
> match the
> rest instead of 'i'.
>
> >
> > The implementation issue is the 2kb buffer-size limit
> imposed in
> > mm-serial-port.c. With a bunch of test messages on my
> device, I'm
> > already up past 3kb of data returned from a single AT+CMGL
> command. A
> > 30-message memory could be up to 10kb with maximum-size
> messages, and
> > I have no reason to think that there aren't devices with
> larger
> > memories. At the very least, I'll cook up a patch to raise
> the buffer
> > size limit, perhaps to 64k; ideally, the stack-allocated
> read buf size
> > wouldn't have to increase as well, since most commands don't
> need this
> > much space. Alternately, it would be nice to have some other
> means of
> > detecting out-of-control modem spew besides an arbitrary
> size limit.
> > Suggestions?
>
>
> Just took a look at the code and that's my read of it too.
> The
> out-of-control thing is only required for probing, so perhaps
> we could
> add another GObject property for "spew-control" (boolean) and
> when
> probing we'd set that property to TRUE, but normally it would
> be FALSE.
> THen we'd check that in data_available() like so:
>
>/* Make sure the response doesn't grow too long */
>if (priv->response->len > SERIAL_BUF_SIZE &&
> priv->spew_control) {
>
> We're certainly not going to be grabbing messages when probing
> the modem
> so the two operations here are mutually exclusive. Note that
> priv->response will grow automatically in size so I don't
> think we need
> to adjust the initial 500 byte size at all.
>
> If you're not entirely familiar with GObject properties yet,
> here's what
> you do...
>
> 1) add the property string name in mm-serial-port.h near the
> top; I like
> to use the #defined names instead of strings to ensure that
> wrong
> property names are compile errors instead of runtime
Re: ModemManager: SMS List API doesn't have an index, buffer isn't big enough
OK, the spew-control property is easy enough; see attached. However, being
new to this codebase, I'm not sure where probing is happening and where I
should be toggling this property on.
- Nathan
On Wed, Apr 6, 2011 at 7:21 PM, Dan Williams wrote:
> On Wed, 2011-04-06 at 12:16 -0400, Nathan Williams wrote:
> > Hi, all. In the process of implementing some SMS support, I've run
> > into a couple of issues with the
> > org.freedesktop.ModemManager.Modem.Gsm.SMS.List DBus call, one in the
> > API and one in the implementation.
>
> Excellent :) We can change the API because for all practical purposes,
> it's not being used yet.
>
> > The API issue is that as specified, the caller gets the contents of
> > all of the messages, but not their index numbers, so there's no way to
> > delete the messages that have been received. Here are three ways I've
> > considered fixing this:
> >
> >
> > 1. Change to a List command that just gets the index numbers of the
> > messages and lets the caller Get and Delete from that as needed. This
> > is an OK API but doesn't really match the AT commands and would be a
> > bit inefficient in that regard.
> > 2. Change the return type from "aa{sv}" to something that includes
> > the index distinct from the message, such as "a(ia{sv})". Reflects the
> > low level well, creates a bit of extra assembly and disassembly work.
> > 3. Add an "index" integer element to the dictionary of each message.
> > This is both easy and still pretty closely reflects the low-level
> > operation.
> >
> >
> > I'm leaning towards #3.
>
> Yeah, I agree. #2 maps more closely to the other methods since they all
> take top-level "u" arguments, but having it in the dict isn't a problem.
> But make sure you make the 'index' item type uint32 ("u") to match the
> rest instead of 'i'.
>
> >
> > The implementation issue is the 2kb buffer-size limit imposed in
> > mm-serial-port.c. With a bunch of test messages on my device, I'm
> > already up past 3kb of data returned from a single AT+CMGL command. A
> > 30-message memory could be up to 10kb with maximum-size messages, and
> > I have no reason to think that there aren't devices with larger
> > memories. At the very least, I'll cook up a patch to raise the buffer
> > size limit, perhaps to 64k; ideally, the stack-allocated read buf size
> > wouldn't have to increase as well, since most commands don't need this
> > much space. Alternately, it would be nice to have some other means of
> > detecting out-of-control modem spew besides an arbitrary size limit.
> > Suggestions?
>
> Just took a look at the code and that's my read of it too. The
> out-of-control thing is only required for probing, so perhaps we could
> add another GObject property for "spew-control" (boolean) and when
> probing we'd set that property to TRUE, but normally it would be FALSE.
> THen we'd check that in data_available() like so:
>
>/* Make sure the response doesn't grow too long */
>if (priv->response->len > SERIAL_BUF_SIZE && priv->spew_control) {
>
> We're certainly not going to be grabbing messages when probing the modem
> so the two operations here are mutually exclusive. Note that
> priv->response will grow automatically in size so I don't think we need
> to adjust the initial 500 byte size at all.
>
> If you're not entirely familiar with GObject properties yet, here's what
> you do...
>
> 1) add the property string name in mm-serial-port.h near the top; I like
> to use the #defined names instead of strings to ensure that wrong
> property names are compile errors instead of runtime ones
>
> 2) add the internal property number at the top of mm-serial-port.c in
> the property enum
>
> 3) in mm_serial_port_class_init() register the property, you'll be using
> g_param_spec_boolean, and make the default be FALSE. You'll want
> G_PARAM_CONSTRUCT_ONLY and G_PARAM_READWRITE for flags since we only
> ever want to set the property when we create the port.
>
> 4) add the 'gboolean spew_control' member to the NMSerialPortPrivate
> struct
>
> 5) then you add the property to get_property() and set_property() using
> g_value_set_boolean() and g_value_get_boolean() respectively, and hook
> up the code to set or return priv->spew_control accordingly
>
> 6) then you can use priv->spew_control to turn it off in
> data_available().
>
> If you knew all that already, my apologies... :) Sounds good to me!
>
> Dan
>
>
>
mm-patch-spew-control
Description: Binary data
___
networkmanager-list mailing list
networkmanager-list@gnome.org
http://mail.gnome.org/mailman/listinfo/networkmanager-list
Re: ModemManager: SMS List API doesn't have an index, buffer isn't big enough
On Wed, 2011-04-06 at 18:21 -0500, Dan Williams wrote:
> On Wed, 2011-04-06 at 12:16 -0400, Nathan Williams wrote:
> > Hi, all. In the process of implementing some SMS support, I've run
> > into a couple of issues with the
> > org.freedesktop.ModemManager.Modem.Gsm.SMS.List DBus call, one in the
> > API and one in the implementation.
>
> Excellent :) We can change the API because for all practical purposes,
> it's not being used yet.
Also on the CDMA side, the way to get at messaging isn't really
standardized. Huawei CDMA modems (EC168C for example) may try to map it
to GSM-type commands, but in general it's available on CDMA phones or
devices unless you start doing QCDM/Diag and poke around the EFS
folders. Of course devices like Gobi may provide access to SMS-related
stuff from the helper library. But if we need to change the API at all
it might be helpful to make it more generic so that CDMA could use it
too.
At least CDMA is fairly simple, without the complexity of multi-part
messages and much of the PDU mode/SMSC stuff.
Dan
> > The API issue is that as specified, the caller gets the contents of
> > all of the messages, but not their index numbers, so there's no way to
> > delete the messages that have been received. Here are three ways I've
> > considered fixing this:
> >
> >
> > 1. Change to a List command that just gets the index numbers of the
> > messages and lets the caller Get and Delete from that as needed. This
> > is an OK API but doesn't really match the AT commands and would be a
> > bit inefficient in that regard.
> > 2. Change the return type from "aa{sv}" to something that includes
> > the index distinct from the message, such as "a(ia{sv})". Reflects the
> > low level well, creates a bit of extra assembly and disassembly work.
> > 3. Add an "index" integer element to the dictionary of each message.
> > This is both easy and still pretty closely reflects the low-level
> > operation.
> >
> >
> > I'm leaning towards #3.
>
> Yeah, I agree. #2 maps more closely to the other methods since they all
> take top-level "u" arguments, but having it in the dict isn't a problem.
> But make sure you make the 'index' item type uint32 ("u") to match the
> rest instead of 'i'.
>
> >
> > The implementation issue is the 2kb buffer-size limit imposed in
> > mm-serial-port.c. With a bunch of test messages on my device, I'm
> > already up past 3kb of data returned from a single AT+CMGL command. A
> > 30-message memory could be up to 10kb with maximum-size messages, and
> > I have no reason to think that there aren't devices with larger
> > memories. At the very least, I'll cook up a patch to raise the buffer
> > size limit, perhaps to 64k; ideally, the stack-allocated read buf size
> > wouldn't have to increase as well, since most commands don't need this
> > much space. Alternately, it would be nice to have some other means of
> > detecting out-of-control modem spew besides an arbitrary size limit.
> > Suggestions?
>
> Just took a look at the code and that's my read of it too. The
> out-of-control thing is only required for probing, so perhaps we could
> add another GObject property for "spew-control" (boolean) and when
> probing we'd set that property to TRUE, but normally it would be FALSE.
> THen we'd check that in data_available() like so:
>
> /* Make sure the response doesn't grow too long */
> if (priv->response->len > SERIAL_BUF_SIZE && priv->spew_control) {
>
> We're certainly not going to be grabbing messages when probing the modem
> so the two operations here are mutually exclusive. Note that
> priv->response will grow automatically in size so I don't think we need
> to adjust the initial 500 byte size at all.
>
> If you're not entirely familiar with GObject properties yet, here's what
> you do...
>
> 1) add the property string name in mm-serial-port.h near the top; I like
> to use the #defined names instead of strings to ensure that wrong
> property names are compile errors instead of runtime ones
>
> 2) add the internal property number at the top of mm-serial-port.c in
> the property enum
>
> 3) in mm_serial_port_class_init() register the property, you'll be using
> g_param_spec_boolean, and make the default be FALSE. You'll want
> G_PARAM_CONSTRUCT_ONLY and G_PARAM_READWRITE for flags since we only
> ever want to set the property when we create the port.
>
> 4) add the 'gboolean spew_control' member to the NMSerialPortPrivate
> struct
>
> 5) then you add the property to get_property() and set_property() using
> g_value_set_boolean() and g_value_get_boolean() respectively, and hook
> up the code to set or return priv->spew_control accordingly
>
> 6) then you can use priv->spew_control to turn it off in
> data_available().
>
> If you knew all that already, my apologies... :) Sounds good to me!
>
> Dan
>
>
> ___
> networkmanager-list mailing list
> networkmanager-list@gnome.org
> http://mail.gnome.org/mailman/
Re: ModemManager: SMS List API doesn't have an index, buffer isn't big enough
On Wed, 2011-04-06 at 12:16 -0400, Nathan Williams wrote:
> Hi, all. In the process of implementing some SMS support, I've run
> into a couple of issues with the
> org.freedesktop.ModemManager.Modem.Gsm.SMS.List DBus call, one in the
> API and one in the implementation.
Excellent :) We can change the API because for all practical purposes,
it's not being used yet.
> The API issue is that as specified, the caller gets the contents of
> all of the messages, but not their index numbers, so there's no way to
> delete the messages that have been received. Here are three ways I've
> considered fixing this:
>
>
> 1. Change to a List command that just gets the index numbers of the
> messages and lets the caller Get and Delete from that as needed. This
> is an OK API but doesn't really match the AT commands and would be a
> bit inefficient in that regard.
> 2. Change the return type from "aa{sv}" to something that includes
> the index distinct from the message, such as "a(ia{sv})". Reflects the
> low level well, creates a bit of extra assembly and disassembly work.
> 3. Add an "index" integer element to the dictionary of each message.
> This is both easy and still pretty closely reflects the low-level
> operation.
>
>
> I'm leaning towards #3.
Yeah, I agree. #2 maps more closely to the other methods since they all
take top-level "u" arguments, but having it in the dict isn't a problem.
But make sure you make the 'index' item type uint32 ("u") to match the
rest instead of 'i'.
>
> The implementation issue is the 2kb buffer-size limit imposed in
> mm-serial-port.c. With a bunch of test messages on my device, I'm
> already up past 3kb of data returned from a single AT+CMGL command. A
> 30-message memory could be up to 10kb with maximum-size messages, and
> I have no reason to think that there aren't devices with larger
> memories. At the very least, I'll cook up a patch to raise the buffer
> size limit, perhaps to 64k; ideally, the stack-allocated read buf size
> wouldn't have to increase as well, since most commands don't need this
> much space. Alternately, it would be nice to have some other means of
> detecting out-of-control modem spew besides an arbitrary size limit.
> Suggestions?
Just took a look at the code and that's my read of it too. The
out-of-control thing is only required for probing, so perhaps we could
add another GObject property for "spew-control" (boolean) and when
probing we'd set that property to TRUE, but normally it would be FALSE.
THen we'd check that in data_available() like so:
/* Make sure the response doesn't grow too long */
if (priv->response->len > SERIAL_BUF_SIZE && priv->spew_control) {
We're certainly not going to be grabbing messages when probing the modem
so the two operations here are mutually exclusive. Note that
priv->response will grow automatically in size so I don't think we need
to adjust the initial 500 byte size at all.
If you're not entirely familiar with GObject properties yet, here's what
you do...
1) add the property string name in mm-serial-port.h near the top; I like
to use the #defined names instead of strings to ensure that wrong
property names are compile errors instead of runtime ones
2) add the internal property number at the top of mm-serial-port.c in
the property enum
3) in mm_serial_port_class_init() register the property, you'll be using
g_param_spec_boolean, and make the default be FALSE. You'll want
G_PARAM_CONSTRUCT_ONLY and G_PARAM_READWRITE for flags since we only
ever want to set the property when we create the port.
4) add the 'gboolean spew_control' member to the NMSerialPortPrivate
struct
5) then you add the property to get_property() and set_property() using
g_value_set_boolean() and g_value_get_boolean() respectively, and hook
up the code to set or return priv->spew_control accordingly
6) then you can use priv->spew_control to turn it off in
data_available().
If you knew all that already, my apologies... :) Sounds good to me!
Dan
___
networkmanager-list mailing list
networkmanager-list@gnome.org
http://mail.gnome.org/mailman/listinfo/networkmanager-list
ModemManager: SMS List API doesn't have an index, buffer isn't big enough
Hi, all. In the process of implementing some SMS support, I've run into a
couple of issues with the org.freedesktop.ModemManager.Modem.Gsm.SMS.List
DBus call, one in the API and one in the implementation.
The API issue is that as specified, the caller gets the contents of all of
the messages, but not their index numbers, so there's no way to delete the
messages that have been received. Here are three ways I've considered fixing
this:
1. Change to a List command that just gets the index numbers of the
messages and lets the caller Get and Delete from that as needed. This is an
OK API but doesn't really match the AT commands and would be a bit
inefficient in that regard.
2. Change the return type from "aa{sv}" to something that includes the
index distinct from the message, such as "a(ia{sv})". Reflects the low level
well, creates a bit of extra assembly and disassembly work.
3. Add an "index" integer element to the dictionary of each message. This
is both easy and still pretty closely reflects the low-level operation.
I'm leaning towards #3.
The implementation issue is the 2kb buffer-size limit imposed in
mm-serial-port.c. With a bunch of test messages on my device, I'm already up
past 3kb of data returned from a single AT+CMGL command. A 30-message memory
could be up to 10kb with maximum-size messages, and I have no reason to
think that there aren't devices with larger memories. At the very least,
I'll cook up a patch to raise the buffer size limit, perhaps to 64k;
ideally, the stack-allocated read buf size wouldn't have to increase as
well, since most commands don't need this much space. Alternately, it would
be nice to have some other means of detecting out-of-control modem spew
besides an arbitrary size limit. Suggestions?
- Nathan
___
networkmanager-list mailing list
networkmanager-list@gnome.org
http://mail.gnome.org/mailman/listinfo/networkmanager-list
