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 <[email protected]> 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 [email protected] http://mail.gnome.org/mailman/listinfo/networkmanager-list
