New 'u-blox' plugin (review)

Dan Williams dcbw at redhat.com
Wed Sep 14 22:34:33 UTC 2016


Hi Aleksander,

I'll put in another recommendation for g_clear_object() :)

> ublox: new +UIPADDR=N response parser
> ublox: new +CGCONTRDP=N response parser
> ublox: implement connection using the 2G/3G logic in router or bridge

It seems like the +CGCONTRDP parsing code could be generic and not
limited to ublox.  However, it looks like the ublox you have (and thus
the code) uses a newer ETSI 27.007 verison of +CGCONTRDP responses.
 Somewhere between v9 and v11 they changed the format (surprise!) and 

Could you add an expected failure testcase for the old format that
ensures we don't succeed in parsing the old format until we explicitly
add support for it?

I got really confused about UIPADDR vs. CGCONTRDP until I read some
+UIPADDR documentation (AT-CommandsExamples_AppNote_(UBX-
13001820).pdf).  That's probably on my list of dumbest things I've
seen.  The CGCONTRDP is supposed to supply the gateway address, but
from the examples it looks like that's always the same as the reported
IP address, while the completely different address from UIPADDR is the
acutal gateway address?  Wow.  This at least deserves some
documentation in the mm-broadband-bearer-ublox.c.


> modem-helpers: new COPS? response parser

Would be nice to have a testcase for the Option hex-encoded COPS case,
here's some sample output for you:

AT+CSCS="IRA"
OK
AT+COPS=3,0;+COPS?
+COPS: 0,0,"T-Mobile",2

OK
AT+CSCS="UCS2"
OK
AT+COPS=3,0;+COPS?
+COPS: 0,0,"0054002d004d006f00620069006c0065",2

OK


> ublox: if on LTE and empty APN given, use the default LTE bearer

The disconnect code reads oddly here; LTE never gets an explicit
disconnect?  That doesn't match the commit message.  My read is that if
an APN was given and the access tech is LTE that APN won't get
disconnected, which doesn't seem correct.

Also, we need better handling of the default bearer in general.  Any
idea how other devices handle this?  My Huawei E3276 doesn't auto-
activate PDP context 0 (shown via +CGACT) when it's registered to LTE.
 Is the ublox special in this regard?

Also, I'm not really sure about this in general.  Usually a blank APN
means "subscription value is requested", eg let the network figure it
out.  That isn't necessarily the Default EPS Bearer AFAICT.

Maybe for LTE if we can detect there is a Default EPS Bearer active we
just create an MMBearer for it by default?  27.007 is a bit confusing
on the relationship between the "initial PDP context" and the "Default
EPS Bearer"; I don't think the two are the same thing, but they both
get activated automatically on network attach.


> ublox: subclass the connection monitoring logic

How about we do this monitoring for all modems?  I was playing around
the other day with a Sierra MC8781 and the only thing that reported
disconnect when I removed antennas was AT+CGACT polling.  It seems
useful for all modems really.


> ublox: new +CFUN? response parser

Seems hard to believe we don't already have a CFUN parser somewhere?
 But we don't... I'd put this into modem helpers and then use it in
linktop, mbm, ublox, and core.


> ublox: custom parsing for CREG/CGREG/CEREG registration state values

These CEREG values are standard ETSI 27.007 CEREG <stat> values, so
can't this just be done in the common parser?


> ublox: new +CESQ response parser
> ublox: new helper to parse +CESQ response into MMSignal objects

Also standard 27.007, can it be generic for all modems?


> ublox: do not try to authenticate or activate the default LTE bearer
context

There's gotta be a way to figure out which PDP context the default
bearer is using instead of assuming this...

> ublox: implement supported bands loading

I guess +UBANDSEL=? doesn't report anything useful?


> ublox: new +UBANDSEL? response parser

Maybe a use-case for the new mm_parse_uint_list()?


Dan


More information about the ModemManager-devel mailing list