Re: [MM] [PATCH 4/5] novatel: Poll for whether the connection still exists.
On Sun, May 6, 2012 at 11:42 AM, Aleksander Morgado aleksan...@lanedo.comwrote: Given that the $NWQMISTATUS reply is not bearer-specific (reports overall status of the modem), I was thinking in moving the periodic connectivity check to MMIfaceModem instead, and plug it in using the same logic as e.g. the signal quality check. In that way, other implementations (e.g. Samsung) could benefit from the same generic timeout handling logic. What do you think? Is it useful to other implementations? I consider the periodic connectivity check a hack to work around Novatel's fairly broken firmware and complete lack of unsolicited notifications. If other modems are also bad at this, that seems fine, but I'd like to resist generalizing features when there's really only one instance of the problem. - Nathan ___ networkmanager-list mailing list networkmanager-list@gnome.org http://mail.gnome.org/mailman/listinfo/networkmanager-list
Re: [MM] [PATCH 4/5] novatel: Poll for whether the connection still exists.
On Mon, May 7, 2012 at 10:37 AM, Aleksander Morgado aleksan...@lanedo.comwrote: Given that the $NWQMISTATUS reply is not bearer-specific (reports overall status of the modem), I was thinking in moving the periodic connectivity check to MMIfaceModem instead, and plug it in using the same logic as e.g. the signal quality check. In that way, other implementations (e.g. Samsung) could benefit from the same generic timeout handling logic. What do you think? Is it useful to other implementations? I consider the periodic connectivity check a hack to work around Novatel's fairly broken firmware and complete lack of unsolicited notifications. If other modems are also bad at this, that seems fine, but I'd like to resist generalizing features when there's really only one instance of the problem. Oh, I thought your Samsung plugin also needed this; may have overlooked it. My Samsung plugin also notifies the core about disconnection events, which may be what you are remembering. In that case it's in response to an unsolicited message, though; it doesn't have to poll. - Nathan ___ networkmanager-list mailing list networkmanager-list@gnome.org http://mail.gnome.org/mailman/listinfo/networkmanager-list
Re: [mm][PATCH 1/5] novatel: Add support for connecting to specific APNs and specifying username/password.
+networkmanager-list On Wed, May 2, 2012 at 10:43 AM, Nathan Williams n...@google.com wrote: On 05/01/2012 06:07 PM, Nathan Williams wrote: Subject: [PATCH 1/5] novatel: Add support for connecting to specific APNs and specifying username/password. While here, pass through the other relevant bearer properties. Did you have time to check the 'bearer-properties' branch I wrote about some time ago? That branch makes it unnecessary to have bearers implement user/password properties themselves. It certainly seemed like a good idea, and would simplify this, but I didn't think it had been merged. I hope you hadn't been waiting on me +apn = mm_at_serial_port_quote_string (mm_broadband_bearer_get_3gpp_apn (bearer)); +user = mm_at_serial_port_quote_string (self-priv-user); +password = mm_at_serial_port_quote_string (self-priv-password); +command = g_strdup_printf ($NWQMICONNECT=,,%s,,,%s,%s, + apn, user, password); mm_at_serial_port_quote_string() is not in git master yet. I remember a thread talking about quoting strings, but don't recall getting a patch about it. Could you also post that one? Will do. I had forgotten that I was using it in both of my plugins, and meant to include it in the stack with my Samsung changes. - Nathan ___ networkmanager-list mailing list networkmanager-list@gnome.org http://mail.gnome.org/mailman/listinfo/networkmanager-list
Re: [MM] [PATCH 2/5] novatel: Use the ALLOWED_SINGLE_AT property. Saves 5 seconds on probing.
On Wed, May 2, 2012 at 6:11 AM, Aleksander Morgado aleksan...@lanedo.comwrote: On 05/01/2012 06:07 PM, Nathan Williams wrote: Date: Thu, 22 Mar 2012 16:18:24 -0400 Subject: [PATCH 2/5] novatel: Use the ALLOWED_SINGLE_AT property. Saves 5 seconds on probing. Is it that the modem exposes only one AT port? Or that it exposes multiple ports but only one is needed? The property was meant for the former case only, not for the latter. With this modem only one of the four exposed serial ports is an AT port. I believe the remaining ones are two Qualcomm things and GPS. When SINGLE_AT is set, _AT is not needed; it's one or the other. OK, will revise. - Nathan ___ networkmanager-list mailing list networkmanager-list@gnome.org http://mail.gnome.org/mailman/listinfo/networkmanager-list
Re: [MM] [PATCH 5/5] novatel: Remove band-setting implementation to improve connections
I'm leery of including code that can run the $NWBAND command to set the modem's state, since as far as I can tell it simply breaks the modem, at least until it is power-cycled. Not setting the bands in the simple connect process unless explicitly requested seems like a fine change on its own, but I think this is too dangerous to include. - Nathan On Wed, May 2, 2012 at 6:29 AM, Aleksander Morgado aleksan...@lanedo.comwrote: On 05/01/2012 06:09 PM, Nathan Williams wrote: Subject: [PATCH 5/5] novatel: Remove band-setting implementation to improve connections The $NWBAND command seems to disturb the internal state of the modem such that it is unlikely to connect to the network, and produces widely varying error codes. We don't actively use this functionality, it's just that setting use all bands is part of the normal ModemManager simple-connect sequence. Remove it so it doesn't get triggered. I would leave this code around, and instead make sure that if no bands-specific configuration is given in the simple connect properties, we don't try to do anything with the bands setup. What do you think? -- Aleksander ___ networkmanager-list mailing list networkmanager-list@gnome.org http://mail.gnome.org/mailman/listinfo/networkmanager-list
[MM] [PATCH] Add a utility routine to do ITU V.250 quoting of strings for AT commands.
Needed by the Novatel plugin. - Nathan 0001-Add-a-utility-routine-to-do-ITU-V.250-quoting-of-str.patch Description: Binary data ___ networkmanager-list mailing list networkmanager-list@gnome.org http://mail.gnome.org/mailman/listinfo/networkmanager-list
Re: [MM] [PATCH 2/5] novatel: Use the ALLOWED_SINGLE_AT property. Saves 5 seconds on probing.
Updated patch attached. - Nathan On Wed, May 2, 2012 at 11:25 AM, Aleksander Morgado aleksan...@lanedo.comwrote: On 05/02/2012 04:53 PM, Nathan Williams wrote: On Wed, May 2, 2012 at 6:11 AM, Aleksander Morgado aleksan...@lanedo.com mailto:aleksan...@lanedo.com wrote: On 05/01/2012 06:07 PM, Nathan Williams wrote: Date: Thu, 22 Mar 2012 16:18:24 -0400 Subject: [PATCH 2/5] novatel: Use the ALLOWED_SINGLE_AT property. Saves 5 seconds on probing. Is it that the modem exposes only one AT port? Or that it exposes multiple ports but only one is needed? The property was meant for the former case only, not for the latter. With this modem only one of the four exposed serial ports is an AT port. I believe the remaining ones are two Qualcomm things and GPS. Ah, good then. When SINGLE_AT is set, _AT is not needed; it's one or the other. OK, will revise. Great, thanks. -- Aleksander 0001-novatel-Use-the-ALLOWED_SINGLE_AT-property.-Saves-5-.patch Description: Binary data ___ networkmanager-list mailing list networkmanager-list@gnome.org http://mail.gnome.org/mailman/listinfo/networkmanager-list
Re: [MM] [PATCH 4/5] novatel: Poll for whether the connection still exists.
Excellent point. Revised. - Nathan On Wed, May 2, 2012 at 6:24 AM, Aleksander Morgado aleksan...@lanedo.comwrote: On 05/01/2012 06:09 PM, Nathan Williams wrote: +static gboolean +poll_connection (MMBroadbandBearerNovatel *bearer) +{ +mm_base_modem_at_command ( +bearer-priv-connection_modem, +$NWQMISTATUS, +3, +FALSE, +(GAsyncReadyCallback)poll_connection_ready, +bearer); +return TRUE; +} Why the extra connection_modem object in the private struct? There's already a proper reference to the modem object in the MMBearer parent, which can be retrieved getting the MM_BEARER_MODEM property. -- Aleksander 0001-novatel-Poll-for-whether-the-connection-still-exists.patch Description: Binary data ___ networkmanager-list mailing list networkmanager-list@gnome.org http://mail.gnome.org/mailman/listinfo/networkmanager-list
[mm][PATCH 1/5] novatel: Add support for connecting to specific APNs and specifying username/password.
This is the first in a series of patches to submit the code we've been using on ChromiumOS. - Nathan 0001-novatel-Add-support-for-connecting-to-specific-APNs-.patch Description: Binary data ___ networkmanager-list mailing list networkmanager-list@gnome.org http://mail.gnome.org/mailman/listinfo/networkmanager-list
[MM] [PATCH 2/5] novatel: Use the ALLOWED_SINGLE_AT property. Saves 5 seconds on probing.
0002-novatel-Use-the-ALLOWED_SINGLE_AT-property.-Saves-5-.patch Description: Binary data ___ networkmanager-list mailing list networkmanager-list@gnome.org http://mail.gnome.org/mailman/listinfo/networkmanager-list
[MM] [PATCH 3/5] novatel: Implement load_access_technologies.
0003-novatel-Implement-load_access_technologies.patch Description: Binary data ___ networkmanager-list mailing list networkmanager-list@gnome.org http://mail.gnome.org/mailman/listinfo/networkmanager-list
[MM] [PATCH 4/5] novatel: Poll for whether the connection still exists.
0004-novatel-Poll-for-whether-the-connection-still-exists.patch Description: Binary data ___ networkmanager-list mailing list networkmanager-list@gnome.org http://mail.gnome.org/mailman/listinfo/networkmanager-list
[MM] [PATCH 5/5] novatel: Remove band-setting implementation to improve connections
0005-novatel-Remove-band-setting-implementation-to-improv.patch Description: Binary data ___ networkmanager-list mailing list networkmanager-list@gnome.org http://mail.gnome.org/mailman/listinfo/networkmanager-list
[ModemManager] Two SMS bugs in the new code
1. When a new single-part message arrives, an Added signal is sent but the Completed signal is not sent. 2. Multipart messages don't seem to work - the multipart bit isn't getting set correctly somewhere?. See logs: 2012-04-25T16:56:55.500166-07:00 localhost ModemManager[1238]: debug [1335398215.499970] [mm-at-serial-port.c:385] debug_log(): (ttyACM1): -- 'CRLF+CMTI: ME,10CRLF' 2012-04-25T16:56:55.500351-07:00 localhost ModemManager[1238]: debug [1335398215.500199] [mm-serial-port.c:929] mm_serial_port_open(): (ttyACM1) device open count is 2 (open) 2012-04-25T16:56:55.500554-07:00 localhost ModemManager[1238]: debug [1335398215.500420] [mm-at-serial-port.c:385] debug_log(): (ttyACM0): -- 'CRLF+CMTI: ME,10CRLF' 2012-04-25T16:56:55.500781-07:00 localhost ModemManager[1238]: debug [1335398215.500646] [mm-at-serial-port.c:385] debug_log(): (ttyACM1): -- 'AT+CMGR=10CR' 2012-04-25T16:56:55.528526-07:00 localhost ModemManager[1238]: debug [1335398215.528348] [mm-at-serial-port.c:385] debug_log(): (ttyACM1): -- 'CRLF+CMGR: 0,,159CRLF07912160130320F5440B916171056429F521405291650569A00500034C0201A9E8F41C949E83C2207B599E07B1DFEE33885E9ED3' 2012-04-25T16:56:55.529131-07:00 localhost ModemManager[1238]: debug [1335398215.528832] [mm-at-serial-port.c:385] debug_log(): (ttyACM1): -- '41E4F23C7D7697C920FA1B54C697E5E3F4BC0C6AD7D9F434081E96D341E3303C2C4EB3D3F4BC0B94A483E6E8779D4D06CDD1EF3BA80E0785E7A0B7BB0C6A97E7F3F0B9CC02B9DF7450780EA2DFDF2C50780EA2A3CBA0BA9B5C96B3F369F71954768FDFE4B4FB0C9297E1F2F2BCECA6CF41CRLF' 2012-04-25T16:56:55.529514-07:00 localhost ModemManager[1238]: debug [1335398215.529373] [mm-at-serial-port.c:385] debug_log(): (ttyACM1): -- 'CRLFOKCRLF' 2012-04-25T16:56:55.529856-07:00 localhost ModemManager[1238]: debug [1335398215.529719] [mm-broadband-modem.c:4186] sms_part_ready(): Correctly parsed PDU (10) 2012-04-25T16:56:55.530756-07:00 localhost ModemManager[1238]: debug [1335398215.530554] [mm-iface-modem-messaging.c:344] mm_iface_modem_messaging_take_part(): Couldn't take part in SMS list: 'This SMS is not a multipart message' 2012-04-25T16:56:55.530851-07:00 localhost ModemManager[1238]: debug [1335398215.530692] [mm-serial-port.c:969] mm_serial_port_close(): (ttyACM1) device open count is 1 (close) 2012-04-25T16:56:57.392557-07:00 localhost ModemManager[1238]: debug [1335398217.392363] [mm-at-serial-port.c:385] debug_log(): (ttyACM0): -- 'CRLF+CMTI: ME,11CRLF' 2012-04-25T16:56:57.392741-07:00 localhost ModemManager[1238]: debug [1335398217.392588] [mm-serial-port.c:929] mm_serial_port_open(): (ttyACM1) device open count is 2 (open) 2012-04-25T16:56:57.393086-07:00 localhost ModemManager[1238]: debug [1335398217.392845] [mm-at-serial-port.c:385] debug_log(): (ttyACM1): -- 'CRLF+CMTI: ME,11CRLF' 2012-04-25T16:56:57.393325-07:00 localhost ModemManager[1238]: debug [1335398217.393184] [mm-at-serial-port.c:385] debug_log(): (ttyACM1): -- 'AT+CMGR=11CR' 2012-04-25T16:56:57.421656-07:00 localhost ModemManager[1238]: debug [1335398217.421476] [mm-at-serial-port.c:385] debug_log(): (ttyACM1): -- 'CRLF+CMGR: 0,,63CRLF07912160130320F6440B916171056429F521405291651569320500034C0202E9E8301D44479741F0B09C3E0785E56590BCCC0ED3C' 2012-04-25T16:56:57.421966-07:00 localhost ModemManager[1238]: debug [1335398217.421821] [mm-at-serial-port.c:385] debug_log(): (ttyACM1): -- 'B6410FD0D7ABBCBA0B0FB4D4797E52E10CRLFCRLFOKCRLF' 2012-04-25T16:56:57.422362-07:00 localhost ModemManager[1238]: debug [1335398217.422207] [mm-broadband-modem.c:4186] sms_part_ready(): Correctly parsed PDU (11) 2012-04-25T16:56:57.423165-07:00 localhost ModemManager[1238]: debug [1335398217.422971] [mm-iface-modem-messaging.c:344] mm_iface_modem_messaging_take_part(): Couldn't take part in SMS list: 'This SMS is not a multipart message' - Nathan ___ networkmanager-list mailing list networkmanager-list@gnome.org http://mail.gnome.org/mailman/listinfo/networkmanager-list
[MM] [PATCH] Adjust parsing of +CNUM response to permit spaces in the alphanumeric descriptor.
As described. This replaces a regexp of ?\S*? , which doesn't match the string Line 1 because of the embedded whitespace, with a somewhat more elaborate regular expression that does. It also accommodates escaped quotes inside the quoted string. (A more general tokenize v.250 comma-separated response function might be good to have here, but that's a larger project). Fixes the OwnNumbers property on my Novatel E362 with a Verizon SIM. - Nathan 0001-Adjust-parsing-of-CNUM-response-to-permit-spaces-in-.patch Description: Binary data ___ networkmanager-list mailing list networkmanager-list@gnome.org http://mail.gnome.org/mailman/listinfo/networkmanager-list
[MM] [PATCH] mm_modem_{get,dup}_own_numbers(): Fix inverted logic.
Pretty much what it says. With this and the prior patch mmcli -m 0 now shows me the modem's number. - Nathan 0001-mm_modem_-get-dup-_own_numbers-Fix-inverted-logic.patch Description: Binary data ___ networkmanager-list mailing list networkmanager-list@gnome.org http://mail.gnome.org/mailman/listinfo/networkmanager-list
Re: [MM 0.6] Plumb up the SPN display-rule bits
On Fri, Mar 9, 2012 at 8:19 AM, Aleksander Morgado aleksan...@lanedo.comwrote: Hey Nathan, The command to get the SPDI in the code didn't work for me: -- AT+CRSM=176,28621,0,0,255 -- +CRSM: 103,0 OK Ah, bummer. With that change on, the list is properly parsed: $ mmcli -i 0 SIM '/org/freedesktop/ModemManager1/SIM/0' - Properties | imsi : '214040106373910' | id : '8934041110059281105' | operator id : '21404' |operator name : 'Yoigo' | operator SPDI list : '21401,21407' |show PLMN at home : no | show op name roaming : no My operator is Yoigo (21404), and I get 2 operators in the SPDI list: 21401 (Vodafone) and 21407 (Movistar). But, I'm not sure about the flags, I believe I should have gotten a 'yes' in 'show PLMN at home', will check that later. For a quick check, grep for the 28486 query - if 'show PLMN at home' is true, the first byte of the response will be 01; this is reporting that the first byte of the response is 00. A possible fix to handle the case where we don't know how much we can read would be to try to read the first bytes of the record (3 or 4 or 5 just in case) to get the full record length of the record, assuming 1-3 bytes max for the size field, so we read A3 + size encoded in 1 or 2 or 3 bytes + 80); and once we know the whole record size, read the exact list size. Yeah, I thought about this, but had hoped to avoid it (If the record is present at all, there will be a minimum of 7 bytes - A3058003FF - so I think that's the right amount for an initial read). Don't know, but that very first read may fail I guess, if an empty SPDI list is found (not sure if that will be A3028000 or A300) and we try to read more than that and the modem doesn't allow it. On some cards the SPDI just isn't present and the whole thing returns an error (the same can happen with the SPN). It's a tiny bit noisy in the logs, but the correct (empty) results show up in the properties. And a side note; it seems that the SIM initialization sequence is not properly run after getting SIM-PIN unlocked, will take care of that myself. OK, I'm glad this wasn't just me. I'll hold off on reporting my crashes until you hit this. - Nathan ___ networkmanager-list mailing list networkmanager-list@gnome.org http://mail.gnome.org/mailman/listinfo/networkmanager-list
Re: [MM 0.6] Plumb up the SPN display-rule bits
On Fri, Mar 9, 2012 at 9:30 AM, Nathan Williams n...@google.com wrote: On Fri, Mar 9, 2012 at 8:19 AM, Aleksander Morgado aleksan...@lanedo.comwrote: A possible fix to handle the case where we don't know how much we can read would be to try to read the first bytes of the record (3 or 4 or 5 just in case) to get the full record length of the record, assuming 1-3 bytes max for the size field, so we read A3 + size encoded in 1 or 2 or 3 bytes + 80); and once we know the whole record size, read the exact list size. Yeah, I thought about this, but had hoped to avoid it (If the record is present at all, there will be a minimum of 7 bytes - A3058003FF - so I think that's the right amount for an initial read). Here's a version that does the 7-then-more retry. It works on my SIM (retrying from 7 to 34 bytes, even though the entries are all FF); see what you think. - Nathan 0001-Fetch-parse-and-plumb-up-the-SPDI-information-from-t.patch Description: Binary data ___ networkmanager-list mailing list networkmanager-list@gnome.org http://mail.gnome.org/mailman/listinfo/networkmanager-list
Re: [MM 0.6] Plumb up the SPN display-rule bits
Also, if we do expose these two properties we also need to load and expose the SPDI network list, or the properties will be useless. Here's a patch to do that. There's a bit of annoying fighting with gchar ** and gdbus's idea of constness, but it seems reasonable otherwise. It works for me on SIMs with no SPDI information at all and on those with extant empty lists; I'd appreciate a test on a SIM known to have a non-empty list. - Nathan 0001-Fetch-parse-and-plumb-up-the-SPDI-information-from-t.patch Description: Binary data ___ networkmanager-list mailing list networkmanager-list@gnome.org http://mail.gnome.org/mailman/listinfo/networkmanager-list
Fwd: [MM 0.6] Plumb up the SPN display-rule bits
[I accidentially sent this reply just to Dan; I wondered why it hadn't gotten any traction on the list. Resending] So let me think this through... bit 1 is about what to display when the registered network is a well-known network, ie a PLMN known to the SIM. Right. 0 means that the phone does not need to show the registered network name if it's well-known network. They probably should have just said 0 means the opposite of 1, the terminology here is kinda confusing. 1 means that the registered network name is required to be shown if the network is a well-known network. I guess b1=1 would force showing a roaming partner's name instead of the SPN if the roaming partner is known to the SIM? It's in addition to the SPN. When registered on the HPLMN [...] The SP Name shall be displayed (122.101 A.4) Perhaps combining both bits into a RequiredNameDisplay (?) property is the right way to go? We already know what PLMN we're registered with, and we can access the PLMN list to figure out if the registered network is, in fact, a well-known network or not. So here, when the network is not a well-known network, b2=0 means SPN name display is required, while b2=1 means you can display the registered network name. Similarly, here, the registered network name must be displayed, and the bit controls whether to additionally display the SPN. If we did combine both bits into one property, then we'd get the following logic: enum { DISPLAY_UNRESTRICTED = 0, DISPLAY_REGISTERED = 1, DISPLAY_SPN = 2 } DisplayName; display = DISPLAY_UNRESTRICTED if (registered network is well-known) { if (b1 == 1) display = DISPLAY_REGISTERED } else { if (b2 == 0) display = DISPLAY_SPN } And then the UI client can use that hint to figure out whether it is required to display the SPN or the registered name. Thoughts? That logic involves mixing bits read from the SIM (static) with the registered state (dynamic). We could do that, but it would have to be somewhere other than the SIM interface, I think. I'm happier just exporting the bits and punting the logic upstream. Partly I think that's the safer choice since the logic is, as you note, so confusing. - Nathan ___ networkmanager-list mailing list networkmanager-list@gnome.org http://mail.gnome.org/mailman/listinfo/networkmanager-list
Re: [MM 0.6] Plumb up the SPN display-rule bits
+ property name=DisplayRegisteredNetworkNameAtHome type=b access=read / Quoting from the TS: b1=0: display of registered PLMN name not required when registered PLMN is either HPLMN or a PLMN in the service provider PLMN list. b1=1: display of registered PLMN name required when registered PLMN is either HPLMN or a PLMN in the service provider PLMN list. I don't think that DisplayRegisteredNetworkNameAtHome covers everything the property is meant to say. Actually, it hides the main purpose of the property, which is the fact that the registered PLMN name should be displayed even if the network is not the home network, but a network allowed in the PLMN list. E.g. I have a card from the 'Yoigo' operator, and when using 2G my phone connects to the 'Movistar'network: but my phone still needs to display 'Yoigo' instead of 'Movistar'. The fact that the device should display the name of the registered PLMN when in the same home PLMN is quite obvious already. ... The name DisplayOperatorNameWhileRoaming is also not fully clear. 'Roaming' means for the modem/phone 'not in the home network', and that covers both being in a network allowed in the list stored in the SIM card, or really roaming in another not-listed network. Following my example before, when I connect to the Movistar network with a Yoigo SIM card, I will be reported as being roaming, even if the network is one of the allowed networks in the list stored in the SIM card. I don't think this is right - the service provider PLMN list, EF_SPDI, referred to in 31.102 is not the list of all possible networks to roam on; it's a separate list that exists purely for fine-tuning this display. If the card from 'Yoigo' doesn't list 'Movistar', then bit 1 is irrelevant, we always display 'Movistar', and bit 2 controls whether we display 'Yoigo' as well. (If the card from 'Yoigo' did list the 'Movistar' PLMN on its (E)HPLMN list, or has it in the EF_SPDI list, then bit 1 controls whether we need to mention 'Movistar' at all (we always need to mention 'Yoigo' when on a home network)). I admit that the inclusion of the EF_SPDI list in the discussion makes the home/roaming distinction a bit fuzzier (though I have yet to see a non-empty EF_SPDI list), but I think that's roughly the right division. From Annex A.4 of 22.101: When registered on the HPLMN, or one of the PLMN Identifications used for Service Provider Name display: (i) The SP Name shall be displayed; (ii) Display of the PLMN Name is an operators option by setting the appropriate fields in the USIM (i.e. the Service Provider name shall be displayed either in parallel to the PLMN Name or instead of the PLMN Name). When registered on neither the HPLMN, nor one of the PLMN Identifications used for Service Provider Name display: (i) The PLMN name shall be displayed; (ii) Display of the SP Name is an operators option by setting the appropriate fields in the USIM. - Nathan ___ networkmanager-list mailing list networkmanager-list@gnome.org http://mail.gnome.org/mailman/listinfo/networkmanager-list
[MM 0.6] Plumb up the SPN display-rule bits
This is required for properly displaying local-operator-vs-home-provider information, per 3GPP (31.102 section 4.1.2 on EF_SPN and 22.101 Annex A). Suggestions welcome on the long and somewhat unwieldy property/function names. - Nathan 0001-Retrieve-and-plumb-up-the-SIM-SPN-bits-that-set-oper.patch Description: Binary data ___ networkmanager-list mailing list networkmanager-list@gnome.org http://mail.gnome.org/mailman/listinfo/networkmanager-list
[ModemManager] [PATCH] mm_gsm_parse_scan_response(): Improve regex-construction error handling
This was the only code in ModemManager using g_error(), and not appropriately, I think. The codebase is still mixed on how to handle errors compiling regular expressions; other places in this file use g_assert(), while the rest of the code tends to report some kind of parse-failure error. Perhaps this makes sense in terms of startup versus while-running operations? - Nathan ___ networkmanager-list mailing list networkmanager-list@gnome.org http://mail.gnome.org/mailman/listinfo/networkmanager-list
[ModemManager] [PATCH] Fix two bugs with multipart SMS handling: signals and listing.
These errors slipped through my testing last time. (For ChromeOS development, I have rigged up a black-box ModemManager test system under autotest, which creates a fake gudev device and a fake modem that responds to AT commands to test this kind of thing. It might be interesting to adapt it into the MM build system somehow; right now it's part of our big autotest suite. The tests are here: http://git.chromium.org/gitweb/?p=chromiumos/third_party/autotest.git;a=tree;f=client/site_tests/network_ModemManagerSMS http://git.chromium.org/gitweb/?p=chromiumos/third_party/autotest.git;a=tree;f=client/site_tests/network_ModemManagerSMSSignal and use the tools I wrote here: http://git.chromium.org/gitweb/?p=chromiumos/third_party/autotest.git;a=tree;f=client/deps/fakegudev http://git.chromium.org/gitweb/?p=chromiumos/third_party/autotest.git;a=tree;f=client/deps/fakemodem ) - Nathan ___ networkmanager-list mailing list networkmanager-list@gnome.org http://mail.gnome.org/mailman/listinfo/networkmanager-list
Re: [ModemManager] [PATCH] Fix two bugs with multipart SMS handling: signals and listing.
BTW, I think you forgot the patches attached. You're right, of course. Sorry about that. This time for sure. - Nathan From f29a410f4763c1237da7699853579cc0be618a66 Mon Sep 17 00:00:00 2001 From: Nathan Williams n...@chromium.org Date: Tue, 22 Nov 2011 17:55:34 -0500 Subject: [PATCH] Fix two bugs with multipart SMS handling: signals and listing. First, arrange for received/complete signals to be sent by calling cmti_received_has_sms() with the message properties even if the message isn't complete yet. Second, make the operation of the List command's multipart message handling independent of message order by doing one pass to insert the messages into the cache and second pass to retrieve the complete messages. Change-Id: I3dcae940d71aec3ddb65c508675f710d1567b0e2 --- src/mm-generic-gsm.c | 51 - 1 files changed, 37 insertions(+), 14 deletions(-) diff --git a/src/mm-generic-gsm.c b/src/mm-generic-gsm.c index d8ba4be..3443c00 100644 --- a/src/mm-generic-gsm.c +++ b/src/mm-generic-gsm.c @@ -1569,10 +1569,6 @@ cmti_received_has_sms (MMModemGsmSms *modem, guint idx; gboolean complete; -/* - * But how will the 'received', non-complete signal get sent? - * Maybe that should happen earlier. - */ if (properties == NULL) return; @@ -1632,6 +1628,10 @@ cmti_received (MMAtSerialPort *port, sms_get_invoke, G_CALLBACK (cmti_received_has_sms), user_data); +mm_callback_info_set_data (cbinfo, + complete-sms-only, + GUINT_TO_POINTER (FALSE), + NULL); if (priv-sms_fetch_pending != 0) { mm_err(sms_fetch_pending is %d, not 0, priv-sms_fetch_pending); @@ -4660,6 +4660,7 @@ sms_get_done (MMAtSerialPort *port, int rv, status, tpdu_len; guint idx; char pdu[SMS_MAX_PDU_LEN + 1]; +gboolean look_for_complete; idx = priv-sms_fetch_pending; priv-sms_fetch_pending = 0; @@ -4694,12 +4695,18 @@ sms_get_done (MMAtSerialPort *port, simple_uint_value (idx)); sms_cache_insert (info-modem, properties, idx); -/* - * If this is a standalone message, or the key part of a - * multipart message, pass it along, otherwise report that there's - * no such message. - */ -properties = sms_cache_lookup_full (info-modem, properties, info-error); +look_for_complete = GPOINTER_TO_UINT (mm_callback_info_get_data(info, + complete-sms-only)); + +if (look_for_complete == TRUE) { +/* + * If this is a standalone message, or the key part of a + * multipart message, pass it along, otherwise report that there's + * no such message. + */ +properties = sms_cache_lookup_full (info-modem, properties, +info-error); +} if (properties) mm_callback_info_set_data (info, GS_HASH_TAG, properties, (GDestroyNotify) g_hash_table_unref); @@ -4751,6 +4758,10 @@ sms_get (MMModemGsmSms *modem, sms_get_invoke, G_CALLBACK (callback), user_data); +mm_callback_info_set_data (info, + complete-sms-only, + GUINT_TO_POINTER (TRUE), + NULL); port = mm_generic_gsm_get_best_at_port (MM_GENERIC_GSM (modem), info-error); if (!port) { @@ -4938,6 +4949,7 @@ sms_list_done (MMAtSerialPort *port, gpointer user_data) { MMCallbackInfo *info = (MMCallbackInfo *) user_data; +MMGenericGsmPrivate *priv = MM_GENERIC_GSM_GET_PRIVATE (info-modem); GPtrArray *results = NULL; int rv, status, tpdu_len, offset; char *rstr; @@ -4950,6 +4962,8 @@ sms_list_done (MMAtSerialPort *port, if (error) info-error = g_error_copy (error); else { +GHashTableIter iter; +gpointer key, value; results = g_ptr_array_new (); rstr = response-str; @@ -4972,15 +4986,24 @@ sms_list_done (MMAtSerialPort *port, g_hash_table_insert (properties, index, simple_uint_value (idx)); sms_cache_insert (info-modem, properties, idx); -/* Only add complete messages to the results */ -properties = sms_cache_lookup_full (info-modem, properties, info-error); -if (properties) -g_ptr_array_add (results, properties); +/* The cache holds a reference, so we don't need it anymore */ +g_hash_table_unref (properties); } else
Re: [ModemManager] [PATCH] mm_gsm_parse_scan_response(): Improve regex-construction error handling
And this time with the patch. - Nathan On Wed, Nov 30, 2011 at 2:07 PM, Nathan Williams n...@google.com wrote: This was the only code in ModemManager using g_error(), and not appropriately, I think. The codebase is still mixed on how to handle errors compiling regular expressions; other places in this file use g_assert(), while the rest of the code tends to report some kind of parse-failure error. Perhaps this makes sense in terms of startup versus while-running operations? - Nathan From 41c12498c4d3b2c27ccd322e496d7001cfc6047f Mon Sep 17 00:00:00 2001 From: Nathan Williams n...@chromium.org Date: Wed, 30 Nov 2011 14:02:00 -0500 Subject: [PATCH] mm_gsm_parse_scan_response(): Improve regex-construction error handling Change the error handling to be a bit more like what appears to have been intended: if constructing the regex fails, report an error and return. The existing code looked like it was set up to do this, but wasn't quite wired together, and had process-terminating calls (g_error()) followed by other code. Change-Id: I4a7cee8fe01291976edc2e343fcbeb73e882f20b --- src/mm-modem-helpers.c |8 1 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/mm-modem-helpers.c b/src/mm-modem-helpers.c index fe0ba7a..912d9ba 100644 --- a/src/mm-modem-helpers.c +++ b/src/mm-modem-helpers.c @@ -112,9 +112,9 @@ mm_gsm_parse_scan_response (const char *reply, GError **error) * +COPS: (2,,T-Mobile,31026,0),(1,ATT,ATT,310410),0) */ -r = g_regex_new (\\((\\d),([^,\\)]*),([^,\\)]*),([^,\\)]*)[\\)]?,(\\d)\\), G_REGEX_UNGREEDY, 0, NULL); +r = g_regex_new (\\((\\d),([^,\\)]*),([^,\\)]*),([^,\\)]*)[\\)]?,(\\d)\\), G_REGEX_UNGREEDY, 0, err); if (err) { -g_error (Invalid regular expression: %s, err-message); +mm_err (Invalid regular expression: %s, err-message); g_error_free (err); g_set_error_literal (error, MM_MODEM_ERROR, MM_MODEM_ERROR_GENERAL, @@ -141,9 +141,9 @@ mm_gsm_parse_scan_response (const char *reply, GError **error) * +COPS: (2,T - Mobile,,31026),(1,Einstein PCS,,31064),(1,Cingular,,31041),,(0,1,3),(0,2) */ -r = g_regex_new (\\((\\d),([^,\\)]*),([^,\\)]*),([^\\)]*)\\), G_REGEX_UNGREEDY, 0, NULL); +r = g_regex_new (\\((\\d),([^,\\)]*),([^,\\)]*),([^\\)]*)\\), G_REGEX_UNGREEDY, 0, err); if (err) { -g_error (Invalid regular expression: %s, err-message); +mm_err (Invalid regular expression: %s, err-message); g_error_free (err); g_set_error_literal (error, MM_MODEM_ERROR, MM_MODEM_ERROR_GENERAL, -- 1.7.3.1 ___ networkmanager-list mailing list networkmanager-list@gnome.org http://mail.gnome.org/mailman/listinfo/networkmanager-list
[ModemManager] [PATCH] serial: report port-not-open in queueing commands via callback instead of just returning.
This is a followup to my previous change here; the port-not-open situation is better treated as a manageable error condition rather than a programming error. - Nathan From cec1bb628cd736fd0ac251e77e87f970cd78a4fb Mon Sep 17 00:00:00 2001 From: Nathan Williams n...@chromium.org Date: Wed, 23 Nov 2011 16:16:20 -0500 Subject: [PATCH] serial: report port-not-open in queueing commands via callback instead of just returning. This permits routines like mm-generic-gsm.c:simple_get_status() to work again, as their callbacks get the error they are expecting. To make this work, adapt get_csq_done() to handle a NULL response when error is set, and make sure that multiple errors don't step on each other in the mm_callback_info_chain() sequence created by simple_get_status(). Change-Id: Ie3a72b1ce71b7f117e8b1f3da7a406c4d2da9e02 --- src/mm-generic-gsm.c | 14 +++--- src/mm-serial-port.c | 10 +- 2 files changed, 20 insertions(+), 4 deletions(-) diff --git a/src/mm-generic-gsm.c b/src/mm-generic-gsm.c index d7a232e..d8ba4be 100644 --- a/src/mm-generic-gsm.c +++ b/src/mm-generic-gsm.c @@ -4187,7 +4187,7 @@ get_csq_done (MMAtSerialPort *port, gpointer user_data) { MMCallbackInfo *info = (MMCallbackInfo *) user_data; -char *reply = response-str; +char *reply; gboolean parsed = FALSE; /* If the modem has already been removed, return without @@ -4200,6 +4200,7 @@ get_csq_done (MMAtSerialPort *port, goto done; } +reply = response-str; if (!strncmp (reply, +CSQ: , 6)) { /* Got valid reply */ int quality; @@ -5596,6 +5597,9 @@ simple_status_got_signal_quality (MMModem *modem, if (!error) { properties = (GHashTable *) mm_callback_info_get_data (info, SS_HASH_TAG); g_hash_table_insert (properties, signal_quality, simple_uint_value (result)); +} else { +g_clear_error (info-error); +info-error = g_error_copy (error); } mm_callback_info_chain_complete_one (info); } @@ -5612,6 +5616,9 @@ simple_status_got_band (MMModem *modem, if (!error) { properties = (GHashTable *) mm_callback_info_get_data (info, SS_HASH_TAG); g_hash_table_insert (properties, band, simple_uint_value (result)); +} else { +g_clear_error (info-error); +info-error = g_error_copy (error); } mm_callback_info_chain_complete_one (info); } @@ -5631,9 +5638,10 @@ simple_status_got_reg_info (MMModemGsmNetwork *modem, if (!modem || mm_callback_info_check_modem_removed (info)) return; -if (error) +if (error) { +g_clear_error (info-error); info-error = g_error_copy (error); -else { +} else { properties = (GHashTable *) mm_callback_info_get_data (info, SS_HASH_TAG); g_hash_table_insert (properties, registration_status, simple_uint_value (status)); diff --git a/src/mm-serial-port.c b/src/mm-serial-port.c index 2d07214..ad5cd91 100644 --- a/src/mm-serial-port.c +++ b/src/mm-serial-port.c @@ -969,9 +969,17 @@ internal_queue_command (MMSerialPort *self, MMQueueData *info; g_return_if_fail (MM_IS_SERIAL_PORT (self)); -g_return_if_fail (priv-open_count 0); g_return_if_fail (command != NULL); +if (!(priv-open_count 0)) { +GError *error = g_error_new_literal (MM_SERIAL_ERROR, + MM_SERIAL_ERROR_SEND_FAILED, + Sending command failed: device is not enabled); +callback (self, NULL, error, user_data); +g_error_free (error); +return; +} + info = g_slice_new0 (MMQueueData); if (take_command) info-command = command; -- 1.7.3.1 ___ networkmanager-list mailing list networkmanager-list@gnome.org http://mail.gnome.org/mailman/listinfo/networkmanager-list
Re: ModemManager: Huawei PDU encoded USSD response truncated to 112 characters
Right, that's my change. Does the USSD path in the plugin know the length of the text, or does it have to derive the text length from the length of the hex string? If it doesn't know the length, it's going to have the same problem I was seeing with SMS messages prior to this change, where gsm_unpack() couldn't tell if a trailing 7-byte block had 7 or 8 GSM7 characters in it. - Nathan On Wed, Nov 9, 2011 at 5:48 AM, Graham Inggs graham.in...@uct.ac.za wrote: I've found that prior to the following commit: http://cgit.freedesktop.org/ModemManager/ModemManager/commit/src/mm-charsets.c?id=d05c87e4c80f1a56a613241d14de4faeb0a8304a ...gsm_unpack() in src/mm-charsets.c took the number of encoded characters as input, now it takes the number of decoded characters as input. On 07/11/2011 21:50, Graham Inggs wrote: I believe the cause of this problem is that the output string of gsm_unpack() needs to be larger than the input string. We get 8 characters out for every 7 input characters. I have attached a simple patch to /plugins/mm-modem-huawei-gsm.c which results in the full USSD response being returned. I am not sure that this patch is the correct fix. It may be that gsm_unpack() in src/mm-charsets.c needs take the 8/7 growth into account when sizing the output array and returning the unpacked size. Would someone more familiar with src/mm-charsets.c offer some guidance please? On Fri, Nov 4, 2011 at 8:44 AM, Graham Inggsgraham.in...@uct.ac.za wrote: Tested with Huawei E1820. ussdresult = gsmussd.Initiate(*101#) print ussd response: %s % ussdresult ussd response: Balance = R 9.50 Expiry date:02/05/2031.On-Net Minutes = 01:16:00.Free SMS's = 51.Free Data = 100.00 MB.Free Da The full response should have been: Balance = R 9.50 Expiry date:02/05/2031.On-Net Minutes = 01:16:00.Free SMS's = 51.Free Data = 100.00 MB.Free Data = 2007.44 MB. Partial ModemManager log: modem-manager[3316]:debug [1320384816.466178] [mm-at-serial-port.c:298] debug_log(): (ttyUSB0): -- 'AT+CUSD=1,AA182C3602,15CR' modem-manager[3316]:debug [1320384816.504610] [mm-at-serial-port.c:298] debug_log(): (ttyUSB0):-- 'CRLFOKCRLF' modem-manager[3316]:debug [1320384816.818950] [mm-at-serial-port.c:298] debug_log(): (ttyUSB2):-- 'CRLF^RSSI:16CRLF' modem-manager[3316]:debug [1320384816.819316] [mm-at-serial-port.c:298] debug_log(): (ttyUSB2):-- 'CRLF^CSNR:-77,-7CRLF' modem-manager[3316]:debug [1320384818.431324] [mm-at-serial-port.c:298] debug_log(): (ttyUSB2):-- 'CRLF+CUSD: 0,C2303BEC1E97413D90149473D5602050110F4FCBF32072985ED6C1642F58ED2583CD62AEA7BBE52CD341CDB4BB4E2FCF413D102CA68BD9743098CB282F9741D3E6F43407F540B598CB282F9741C4303D0CEA816230980B060335852EA3BC5C0611C3F430A80792C16037178D066A0A5D,15CRLF' ___ networkmanager-list mailing list networkmanager-list@gnome.org http://mail.gnome.org/mailman/listinfo/networkmanager-list ___ networkmanager-list mailing list networkmanager-list@gnome.org http://mail.gnome.org/mailman/listinfo/networkmanager-list
Re: SMS receiving question
Do you have logs from ModemManager? If you run with --log-level=DEBUG, you should see the +CMTI notification when the SMS arrives, and we can see what MM does from that point forward. - Nathan On Fri, Nov 4, 2011 at 6:12 AM, Jean Parpaillon jparpail...@mandriva.com wrote: Hi all, I am using ModemManager to receive SMS with a Huawei E1752 USB modem. I am plugged on signal SmsReceived but I receive nothing. I used ofono stack to do the same things and it worked, but I can not run both ofono and ModemManager at the same time. Do you know if my issue comes from this particular modem or is a known ModemManager issue ? Best regards, -- Jean Parpaillon Pulse2 project leader Mandriva SA - http://mandriva.com Rennes - FR Phone: +33 6 30 10 92 86 email: jparpail...@mandriva.com jabber: jean.parpail...@gmail.com skype: jean.parpaillon ___ networkmanager-list mailing list networkmanager-list@gnome.org http://mail.gnome.org/mailman/listinfo/networkmanager-list ___ networkmanager-list mailing list networkmanager-list@gnome.org http://mail.gnome.org/mailman/listinfo/networkmanager-list
[PATCH 1/2] MM: disable(): Finish all disable commands before returning
This addresses some dubious behavior that was one source of a crash on our systems. - Nathan From 7e9aebaeb179be5a9093c74815af45b62cc849b7 Mon Sep 17 00:00:00 2001 From: Nathan Williams n...@chromium.org Date: Thu, 3 Nov 2011 16:13:07 -0400 Subject: [PATCH 1/2] disable(): Finish all disable commands before returning Rearrange the primary and secondary-port disable operations so that there's a linear chain of callbacks rather than a second dangling callback chain for the secondary port; it's possible for the primary port operations to complete, and for the callback to finish and start tearing down the entire device, before the secondary port commands run. Change-Id: Ia95a7eae574737cdec38b14d39786127be1b3184 --- src/mm-generic-gsm.c | 40 +--- 1 files changed, 25 insertions(+), 15 deletions(-) diff --git a/src/mm-generic-gsm.c b/src/mm-generic-gsm.c index a0eed39..423c8bb 100644 --- a/src/mm-generic-gsm.c +++ b/src/mm-generic-gsm.c @@ -2088,6 +2088,22 @@ disable_flash_done (MMSerialPort *port, g_free (cmd); } +void +mark_disabled (gpointer user_data) +{ +MMCallbackInfo *info = user_data; +MMGenericGsmPrivate *priv = MM_GENERIC_GSM_GET_PRIVATE (info-modem); + +mm_modem_set_state (MM_MODEM (info-modem), +MM_MODEM_STATE_DISABLING, +MM_MODEM_STATE_REASON_NONE); + +if (mm_port_get_connected (MM_PORT (priv-primary))) +mm_serial_port_flash (MM_SERIAL_PORT (priv-primary), 1000, TRUE, disable_flash_done, info); +else +disable_flash_done (MM_SERIAL_PORT (priv-primary), NULL, info); +} + static void secondary_unsolicited_done (MMAtSerialPort *port, GString *response, @@ -2095,6 +2111,7 @@ secondary_unsolicited_done (MMAtSerialPort *port, gpointer user_data) { mm_serial_port_close_force (MM_SERIAL_PORT (port)); +mark_disabled (user_data); } static void @@ -2132,13 +2149,6 @@ disable (MMModem *modem, update_lac_ci (self, 0, 0, 1); _internal_update_access_technology (self, MM_MODEM_GSM_ACCESS_TECH_UNKNOWN); -/* Clean up the secondary port if it's open */ -if (priv-secondary mm_serial_port_is_open (MM_SERIAL_PORT (priv-secondary))) { -mm_at_serial_port_queue_command (priv-secondary, +CREG=0, 3, NULL, NULL); -mm_at_serial_port_queue_command (priv-secondary, +CGREG=0, 3, NULL, NULL); -mm_at_serial_port_queue_command (priv-secondary, +CMER=0, 3, secondary_unsolicited_done, NULL); -} - info = mm_callback_info_new (modem, callback, user_data); /* Cache the previous state so we can reset it if the operation fails */ @@ -2148,14 +2158,14 @@ disable (MMModem *modem, GUINT_TO_POINTER (state), NULL); -mm_modem_set_state (MM_MODEM (info-modem), -MM_MODEM_STATE_DISABLING, -MM_MODEM_STATE_REASON_NONE); - -if (mm_port_get_connected (MM_PORT (priv-primary))) -mm_serial_port_flash (MM_SERIAL_PORT (priv-primary), 1000, TRUE, disable_flash_done, info); -else -disable_flash_done (MM_SERIAL_PORT (priv-primary), NULL, info); +/* Clean up the secondary port if it's open */ +if (priv-secondary mm_serial_port_is_open (MM_SERIAL_PORT (priv-secondary))) { +mm_dbg(Shutting down secondary port); +mm_at_serial_port_queue_command (priv-secondary, +CREG=0, 3, NULL, NULL); +mm_at_serial_port_queue_command (priv-secondary, +CGREG=0, 3, NULL, NULL); +mm_at_serial_port_queue_command (priv-secondary, +CMER=0, 3, secondary_unsolicited_done, info); +} else +mark_disabled (info); } static void -- 1.7.3.1 ___ networkmanager-list mailing list networkmanager-list@gnome.org http://mail.gnome.org/mailman/listinfo/networkmanager-list
[PATCH 2/2] MM: internal_queue_command(): Reject new commands when the port is closed.
This is the actual crasher. - Nathan From 3cd611e30fce4b572ab7f89cca876dff3362cd36 Mon Sep 17 00:00:00 2001 From: Nathan Williams n...@chromium.org Date: Thu, 3 Nov 2011 16:21:45 -0400 Subject: [PATCH 2/2] internal_queue_command(): Reject new commands when the port is closed. Otherwise, we can schedule a main loop call to mm_serial_port_queue_process() for an object that's about to disappear, leading to a crash. Change-Id: I433a76855c52536eb2b99a5ecf26ac71afe1f8bb --- src/mm-serial-port.c |1 + 1 files changed, 1 insertions(+), 0 deletions(-) diff --git a/src/mm-serial-port.c b/src/mm-serial-port.c index b116997..2d07214 100644 --- a/src/mm-serial-port.c +++ b/src/mm-serial-port.c @@ -969,6 +969,7 @@ internal_queue_command (MMSerialPort *self, MMQueueData *info; g_return_if_fail (MM_IS_SERIAL_PORT (self)); +g_return_if_fail (priv-open_count 0); g_return_if_fail (command != NULL); info = g_slice_new0 (MMQueueData); -- 1.7.3.1 ___ networkmanager-list mailing list networkmanager-list@gnome.org http://mail.gnome.org/mailman/listinfo/networkmanager-list
[PATCH] ModemManager: Multipart SMS support
Kind of a complicated patch, but it does the job without mangling the state on the modem, so we're not dependent on ModemManager to generate or store any new state of its own. - Nathan 0001-Multipart-SMS-support.patch Description: Binary data ___ networkmanager-list mailing list networkmanager-list@gnome.org http://mail.gnome.org/mailman/listinfo/networkmanager-list
[PATCH] ModemManager: Clean up GRegex/GMatchInfo leaks
During some other development we found that there was a lot of inconsistency in freeing GMatchInfo structures after g_regex operations, and that quite a few match_info structures were probably being leaked. This patch is the result of a quick audit of the g_regex callers for properly cleaning up the match_info. - Nathan From a090d2f3d99a1bd52fdd652b043e66e7dc48bb2c Mon Sep 17 00:00:00 2001 From: Nathan Williams n...@chromium.org Date: Mon, 26 Sep 2011 13:20:17 -0400 Subject: [PATCH] Ensure that GMatchInfo and GRegex objects are freed properly. In particular, g_regex_match() and g_regex_match_full() allocate a match_info structure on both success and failure, so calling g_match_info_free() only in the success case is insufficient. BUG=None TEST=Inspection Change-Id: Iea76b5b5dc3ec48120e15601a5e2dd45322133d8 --- plugins/mm-modem-anydata-cdma.c |6 ++ plugins/mm-modem-cinterion-gsm.c |9 + plugins/mm-modem-huawei-gsm.c|3 ++- plugins/mm-modem-novatel-gsm.c |3 +-- plugins/mm-modem-samsung-gsm.c |3 ++- plugins/mm-modem-sierra-gsm.c|4 +++- plugins/mm-modem-x22x-gsm.c |3 +-- plugins/mm-modem-zte.c |6 +++--- src/mm-generic-gsm.c |4 src/mm-modem-helpers.c | 15 ++- src/mm-serial-parsers.c | 31 +-- 11 files changed, 50 insertions(+), 37 deletions(-) diff --git a/plugins/mm-modem-anydata-cdma.c b/plugins/mm-modem-anydata-cdma.c index 7b6b37a..d26d3ec 100644 --- a/plugins/mm-modem-anydata-cdma.c +++ b/plugins/mm-modem-anydata-cdma.c @@ -183,6 +183,9 @@ evdo_state_done (MMAtSerialPort *port, } } +g_match_info_free (match_info); +g_regex_unref (r); + done: mm_generic_cdma_query_reg_state_set_callback_evdo_state (info, reg_state); mm_callback_info_schedule (info); @@ -254,6 +257,9 @@ state_done (MMAtSerialPort *port, } } +g_match_info_free (match_info); +g_regex_unref (r); + mm_generic_cdma_query_reg_state_set_callback_1x_state (info, reg_state); /* Try for EVDO state too */ diff --git a/plugins/mm-modem-cinterion-gsm.c b/plugins/mm-modem-cinterion-gsm.c index 56a6b00..b6ea841 100644 --- a/plugins/mm-modem-cinterion-gsm.c +++ b/plugins/mm-modem-cinterion-gsm.c @@ -615,6 +615,9 @@ get_smong_cb (MMAtSerialPort *port, priv-sind_psinfo = TRUE; } +g_match_info_free (match_info); +g_regex_unref (regex); + mm_callback_info_set_result (info, GUINT_TO_POINTER (act), NULL); mm_callback_info_schedule (info); } @@ -661,9 +664,15 @@ get_sind_cb (MMAtSerialPort *port, g_free (ind_value); mm_callback_info_set_result (info, GUINT_TO_POINTER (act), NULL); mm_callback_info_schedule (info); + +g_match_info_free (match_info); +g_regex_unref (regex); return; } +g_match_info_free (match_info); +g_regex_unref (regex); + /* If there was no 'psinfo' indicator, we'll try AT^SMONG and read the cell * info table. */ mm_at_serial_port_queue_command (port, ^SMONG, 3, get_smong_cb, info); diff --git a/plugins/mm-modem-huawei-gsm.c b/plugins/mm-modem-huawei-gsm.c index e038069..f615e19 100644 --- a/plugins/mm-modem-huawei-gsm.c +++ b/plugins/mm-modem-huawei-gsm.c @@ -595,9 +595,10 @@ send_huawei_cpin_done (MMAtSerialPort *port, mm_callback_info_set_result (info, GUINT_TO_POINTER (attempts_left), NULL); -g_match_info_free (match_info); done: +if (match_info) +g_match_info_free (match_info); if (r) g_regex_unref (r); mm_serial_port_close (MM_SERIAL_PORT (port)); diff --git a/plugins/mm-modem-novatel-gsm.c b/plugins/mm-modem-novatel-gsm.c index 5d78db7..706664c 100644 --- a/plugins/mm-modem-novatel-gsm.c +++ b/plugins/mm-modem-novatel-gsm.c @@ -198,8 +198,6 @@ parse_nwrat_response (GString *response, mode = atoi (str); g_free (str); -g_match_info_free (match_info); - if (mode 0 || mode 2) { g_set_error_literal (error, MM_MODEM_ERROR, MM_MODEM_ERROR_GENERAL, Failed to parse mode/tech response); @@ -219,6 +217,7 @@ parse_nwrat_response (GString *response, success = TRUE; out: +g_match_info_free (match_info); g_regex_unref (r); return success; } diff --git a/plugins/mm-modem-samsung-gsm.c b/plugins/mm-modem-samsung-gsm.c index df7a4b4..976d419 100755 --- a/plugins/mm-modem-samsung-gsm.c +++ b/plugins/mm-modem-samsung-gsm.c @@ -357,9 +357,10 @@ send_samsung_pinnum_done (MMAtSerialPort *port, mm_callback_info_set_result (info, GUINT_TO_POINTER (attempts_left), NULL); -g_match_info_free (match_info); done: +if (match_info) +g_match_info_free (match_info); if (r) g_regex_unref (r); mm_serial_port_close (MM_SERIAL_PORT (port)); diff --git a/plugins/mm-modem-sierra-gsm.c b/plugins/mm-modem-sierra-gsm.c index 6d4e4d5..c15598f 100644
Re: PATCH] sms_decode_text(): Sanitize 8-bit data so that it is UTF8-clean.
On Tue, Sep 27, 2011 at 2:18 PM, Dan Williams d...@redhat.com wrote: On Mon, 2011-09-26 at 18:29 -0400, Nathan Williams wrote: This keeps ModemManager from crashing deep in the DBus libraries when a SMS Get() or List() DBus operation finds a message that isn't valid UTF-8 and/or has embedded NUL characters. I'll be putting up a separate patch as a proposal for how to avoid this problem in the new API. Sounds fine; though in general we know the encoding that the message comes in with, and we know we need to convert to UTF-8 for D-Bus (and really, everything should be UTF-8 at the boundaries, it would be just horrid to expose any charset encoding details to clients and I don't think we have to). So we should be able to convert to UTF-8 without any real loss of fidelity when reading the message from the modem, and we should be able to convert from UTF-8 to a suitable charset (whatever we've selected from CSCS) when sending messages too. In what cases would we want to send or receive essentially binary data via SMS? AFAIK most of these cases show up as base64 or hex-string SMS if they aren't intended for human consumption. We do do that conversion to UTF-8 when we know the transmission character set, GSM-7 or UCS2. The one fly in this ointment is that one of the possible encodings is, in fact, 8-bit data (TP-DCS value of 04 or f4) with no associated character set. The particular case that brought this to my attention was a test SMS from a carrier that was supposed to contain, I believe, a polyphonic ringtone for some Nokia handset. I'll see about updating the testcases. - Nathan ___ networkmanager-list mailing list networkmanager-list@gnome.org http://mail.gnome.org/mailman/listinfo/networkmanager-list
PATCH] sms_decode_text(): Sanitize 8-bit data so that it is UTF8-clean.
This keeps ModemManager from crashing deep in the DBus libraries when a SMS Get() or List() DBus operation finds a message that isn't valid UTF-8 and/or has embedded NUL characters. I'll be putting up a separate patch as a proposal for how to avoid this problem in the new API. - Nathan From b4be9e8cfa79cfb1d63e69a151078c75f38131d9 Mon Sep 17 00:00:00 2001 From: Nathan Williams n...@chromium.org Date: Fri, 23 Sep 2011 17:21:15 -0400 Subject: [PATCH] sms_decode_text(): Sanitize 8-bit data so that it is UTF8-clean. When receiving a SMS message with raw 8-bit data, sanitize it by replacing non-ASCII characters with \xNN escape sequences. This prevents a problem further down the line where the body of the message is passed into DBus as a string, and DBus chokes because the string isn't valid UTF-8. Once the ModemManager SMS API can support non-string message bodies, this should be revisited. BUG=chrome-os-partner:5953 TEST=Run network_ModemManagerSMS.py with the PDU from this bug. Change-Id: Ic33a365f9a065c49a325e047e4c3f5e81450fa1f Reviewed-on: http://gerrit.chromium.org/gerrit/8232 Reviewed-by: Eric Shienbrood e...@chromium.org Tested-by: Nathan J. Williams n...@chromium.org Commit-Ready: Nathan J. Williams n...@chromium.org --- src/mm-sms-utils.c | 21 +++-- 1 files changed, 19 insertions(+), 2 deletions(-) diff --git a/src/mm-sms-utils.c b/src/mm-sms-utils.c index 3f56a64..89eae4b 100644 --- a/src/mm-sms-utils.c +++ b/src/mm-sms-utils.c @@ -13,6 +13,9 @@ * Copyright (C) 2011 Red Hat, Inc. */ +#include ctype.h +#include stdio.h + #include glib.h #include mm-charsets.h @@ -200,8 +203,22 @@ sms_decode_text (const guint8 *text, int len, SmsEncoding encoding, int bit_offs g_free (unpacked); } else if (encoding == MM_SMS_ENCODING_UCS2) utf8 = g_convert ((char *) text, len, UTF8, UCS-2BE, NULL, NULL, NULL); -else if (encoding == MM_SMS_ENCODING_8BIT) -utf8 = g_strndup ((const char *)text, len); +else if (encoding == MM_SMS_ENCODING_8BIT) { +/* DBus may choke on non-UTF8 strings, so we have some sanitizing to do */ +char *p; +int i; +utf8 = g_malloc0 (4*len+1); /* Worst case: Every byte becomes \xFF */ +p = utf8; +for (i = 0 ; i len ; i++) { +if (isascii (text[i]) text[i] != '\0') +*p++ = text[i]; +else { +sprintf(p, \\x%02x, text[i]); +p += 4; +} +} +*p = '\0'; +} else utf8 = g_strdup (); -- 1.7.3.1 ___ networkmanager-list mailing list networkmanager-list@gnome.org http://mail.gnome.org/mailman/listinfo/networkmanager-list
[MM] [PATCH] sms_get_done(): Check for the correct return value from sscanf()
The %n format in sscanf() is tricky; it's best avoided if we don't actually need it. (dropped through the cracks of my upstreaming process a little while ago, sorry) - Nathan From fc3f0e40f1c7196f38789f585dca98f22fb13601 Mon Sep 17 00:00:00 2001 From: Nathan Williams n...@chromium.org Date: Fri, 24 Jun 2011 21:12:00 -0400 Subject: [PATCH] sms_get_done(): Check for the correct return value from sscanf(). Remove an unused variable so it's more obvious what the correct value is. Fixes live (vs. list) SMS reception on ToT. BUG=none TEST=Send SMS to device, see that it shows up in Chrome. Change-Id: I9c76fb15ef229fe83672e2eee8ae37d7e6ab7b9e Reviewed-on: http://gerrit.chromium.org/gerrit/3216 Reviewed-by: Nathan J. Williams n...@chromium.org Tested-by: Nathan J. Williams n...@chromium.org --- src/mm-generic-gsm.c |8 1 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/mm-generic-gsm.c b/src/mm-generic-gsm.c index 8e18222..3f7c9ab 100644 --- a/src/mm-generic-gsm.c +++ b/src/mm-generic-gsm.c @@ -4472,7 +4472,7 @@ sms_get_done (MMAtSerialPort *port, { MMCallbackInfo *info = (MMCallbackInfo *) user_data; GHashTable *properties; -int rv, status, tpdu_len, offset; +int rv, status, tpdu_len; char pdu[SMS_MAX_PDU_LEN + 1]; /* If the modem has already been removed, return without @@ -4486,9 +4486,9 @@ sms_get_done (MMAtSerialPort *port, } /* 344 == SMS_MAX_PDU_LEN */ -rv = sscanf (response-str, +CMGR: %d,,%d %344s %n, - status, tpdu_len, pdu, offset); -if (rv != 4) { +rv = sscanf (response-str, +CMGR: %d,,%d %344s, + status, tpdu_len, pdu); +if (rv != 3) { info-error = g_error_new (MM_MODEM_ERROR, MM_MODEM_ERROR_GENERAL, Failed to parse CMGR response (parsed %d items), -- 1.7.3.1 ___ networkmanager-list mailing list networkmanager-list@gnome.org http://mail.gnome.org/mailman/listinfo/networkmanager-list
[ModemManager] [PATCH 1/4] Split SMS parsing out into a separate file
First of four patches that improve SMS handling. This one follows Dan's suggestion some time ago and splits SMS PDU parsing into a separate file, making it accessible to unit tests and to other code that might need it. - Nathan ___ networkmanager-list mailing list networkmanager-list@gnome.org http://mail.gnome.org/mailman/listinfo/networkmanager-list
Re: [ModemManager] [PATCH 1/4] Split SMS parsing out into a separate file
This time with the actual patch. On Thu, Jun 30, 2011 at 5:31 PM, Nathan Williams n...@google.com wrote: First of four patches that improve SMS handling. This one follows Dan's suggestion some time ago and splits SMS PDU parsing into a separate file, making it accessible to unit tests and to other code that might need it. - Nathan From 4c482e8f98b07867849195a9f2d7244787666119 Mon Sep 17 00:00:00 2001 From: Nathan Williams n...@chromium.org Date: Fri, 24 Jun 2011 21:07:24 -0400 Subject: [PATCH 1/4] Split SMS parsing out into a separate file, in preparation for adding some tests. Change-Id: If1ebd0fdd6e7470c21538042ab1735357649155c --- src/Makefile.am |4 +- src/mm-generic-gsm.c | 266 + src/mm-sms-utils.c | 320 ++ src/mm-sms-utils.h | 25 4 files changed, 355 insertions(+), 260 deletions(-) create mode 100644 src/mm-sms-utils.c create mode 100644 src/mm-sms-utils.h diff --git a/src/Makefile.am b/src/Makefile.am index e813e7e..71008c9 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -23,7 +23,9 @@ libmodem_helpers_la_SOURCES = \ mm-charsets.c \ mm-charsets.h \ mm-utils.c \ - mm-utils.h + mm-utils.h \ + mm-sms-utils.c \ + mm-sms-utils.h libserial_la_CPPFLAGS = \ $(MM_CFLAGS) \ diff --git a/src/mm-generic-gsm.c b/src/mm-generic-gsm.c index 6cc01a6..6bbd69b 100644 --- a/src/mm-generic-gsm.c +++ b/src/mm-generic-gsm.c @@ -37,6 +37,7 @@ #include mm-properties-changed-signal.h #include mm-utils.h #include mm-modem-location.h +#include mm-sms-utils.h static void modem_init (MMModem *modem_class); static void modem_gsm_card_init (MMModemGsmCard *gsm_card_class); @@ -177,7 +178,6 @@ static void cmti_received (MMAtSerialPort *port, #define GS_HASH_TAG get-sms static GValue *simple_string_value (const char *str); static GValue *simple_uint_value (guint32 i); -static GValue *simple_boolean_value (gboolean b); static void simple_free_gvalue (gpointer data); MMModem * @@ -4171,35 +4171,6 @@ mm_generic_gsm_get_charset (MMGenericGsm *self) /* MMModemGsmSms interface */ -#define SMS_TP_MTI_MASK 0x03 -#define SMS_TP_MTI_SMS_DELIVER 0x00 -#define SMS_TP_MTI_SMS_SUBMIT_REPORT 0x01 -#define SMS_TP_MTI_SMS_STATUS_REPORT 0x02 - -#define SMS_NUMBER_TYPE_MASK 0x70 -#define SMS_NUMBER_TYPE_UNKNOWN 0x00 -#define SMS_NUMBER_TYPE_INTL 0x10 -#define SMS_NUMBER_TYPE_ALPHA 0x50 - -#define SMS_NUMBER_PLAN_MASK 0x0f -#define SMS_NUMBER_PLAN_TELEPHONE 0x01 - -#define SMS_TP_MMS0x04 -#define SMS_TP_SRI0x20 -#define SMS_TP_UDHI 0x40 -#define SMS_TP_RP 0x80 - -#define SMS_DCS_CODING_MASK 0xec -#define SMS_DCS_CODING_DEFAULT 0x00 -#define SMS_DCS_CODING_8BIT 0x04 -#define SMS_DCS_CODING_UCS2 0x08 - -#define SMS_DCS_CLASS_VALID 0x10 -#define SMS_DCS_CLASS_MASK0x03 - -#define SMS_TIMESTAMP_LEN 7 -#define SMS_MIN_PDU_LEN (7 + SMS_TIMESTAMP_LEN) -#define SMS_MAX_PDU_LEN 344 static void sms_send_done (MMAtSerialPort *port, @@ -4250,219 +4221,7 @@ sms_send (MMModemGsmSms *modem, g_free (command); } -static char sms_bcd_chars[] = 0123456789*#abc\0\0; - -static void -sms_semi_octets_to_bcd_string (char *dest, const guint8 *octets, int num_octets) -{ -int i; - -for (i = 0 ; i num_octets; i++) { -*dest++ = sms_bcd_chars[octets[i] 0xf]; -*dest++ = sms_bcd_chars[(octets[i] 4) 0xf]; -} -*dest++ = '\0'; -} - -/* len is in semi-octets */ -static char * -sms_decode_address (const guint8 *address, int len) -{ -guint8 addrtype, addrplan; -char *utf8; - -addrtype = address[0] SMS_NUMBER_TYPE_MASK; -addrplan = address[0] SMS_NUMBER_PLAN_MASK; -address++; - -if (addrtype == SMS_NUMBER_TYPE_ALPHA) { -guint8 *unpacked; -guint32 unpacked_len; -unpacked = gsm_unpack (address, (len * 4) / 7, 0, unpacked_len); -utf8 = (char *)mm_charset_gsm_unpacked_to_utf8 (unpacked, -unpacked_len); -g_free(unpacked); -} else if (addrtype == SMS_NUMBER_TYPE_INTL - addrplan == SMS_NUMBER_PLAN_TELEPHONE) { -/* International telphone number, format as +1234567890 */ -utf8 = g_malloc (len + 3); /* '+' + digits + possible trailing 0xf + NUL */ -utf8[0] = '+'; -sms_semi_octets_to_bcd_string (utf8 + 1, address, (len + 1) / 2); -} else { -/* - * All non-alphanumeric types and plans are just digits, but - * don't apply any special formatting if we don't know the - * format. - */ -utf8 = g_malloc (len + 2); /* digits + possible trailing 0xf + NUL */ -sms_semi_octets_to_bcd_string (utf8, address, (len + 1) / 2); -} - -return utf8; -} - -static char
[ModemManager] [PATCH 2/4] Add SMS unit tests.
Second in a series. Builds on the previous by actually unit-testing the sms_parse_pdu() function. Note that the dcf1 test does not pass as the code is currently written. - Nathan From ee7476b84a833cecf5999232b1fbd14d0dee Mon Sep 17 00:00:00 2001 From: Nathan Williams n...@chromium.org Date: Wed, 29 Jun 2011 16:52:01 -0400 Subject: [PATCH 2/4] Add SMS unit tests. Change-Id: Ic172df7d67233dc51a2c3acd5439e2f96f8726cf --- src/tests/Makefile.am | 14 ++- src/tests/test-sms.c | 400 + 2 files changed, 413 insertions(+), 1 deletions(-) create mode 100644 src/tests/test-sms.c diff --git a/src/tests/Makefile.am b/src/tests/Makefile.am index e265bc1..cc47e66 100644 --- a/src/tests/Makefile.am +++ b/src/tests/Makefile.am @@ -4,7 +4,8 @@ INCLUDES = \ noinst_PROGRAMS = \ test-modem-helpers \ test-charsets \ - test-qcdm-serial-port + test-qcdm-serial-port \ + test-sms test_modem_helpers_SOURCES = \ test-modem-helpers.c @@ -40,12 +41,23 @@ test_qcdm_serial_port_LDADD = \ $(top_builddir)/libqcdm/src/libqcdm.la \ -lutil +test_sms_SOURCES = \ + test-sms.c + +test_sms_CFLAGS = \ + $(MM_CFLAGS) + +test_sms_LDADD = \ + $(top_builddir)/src/libmodem-helpers.la \ + $(MM_LIBS) + if WITH_TESTS check-local: test-modem-helpers $(abs_builddir)/test-modem-helpers $(abs_builddir)/test-charsets $(abs_builddir)/test-qcdm-serial-port + $(abs_builddir)/test-sms endif diff --git a/src/tests/test-sms.c b/src/tests/test-sms.c new file mode 100644 index 000..dfbb195 --- /dev/null +++ b/src/tests/test-sms.c @@ -0,0 +1,400 @@ +/* -*- Mode: C; tab-width: 4; indent-tabs-mode: nil; c-basic-offset: 4 -*- */ +/* + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; either version 2 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details: + * + * Copyright (C) 2010 Red Hat, Inc. + * Copyright (C) 2011 The Chromium OS Authors. + */ + +#include glib.h +#include glib-object.h +#include string.h + +#include mm-sms-utils.h +#include mm-utils.h + + +#define TEST_ENTRY_EQ(hash, key, expectvalue) do { \ + GValue *value; \ + value = g_hash_table_lookup((hash), (key)); \ + g_assert(value); \ + g_assert(G_VALUE_HOLDS_STRING(value)); \ + g_assert_cmpstr(g_value_get_string(value), ==, (expectvalue)); \ + } while (0) + +static void +test_pdu1 (void *f, gpointer d) +{ + static const guint8 pdu[] = { +0x07, 0x91, 0x21, 0x04, 0x44, 0x29, 0x61, 0xf4, +0x04, 0x0b, 0x91, 0x61, 0x71, 0x95, 0x72, 0x91, +0xf8, 0x00, 0x00, 0x11, 0x20, 0x82, 0x11, 0x05, +0x05, 0x0a, +// user data: +0x6a, 0xc8, 0xb2, 0xbc, 0x7c, 0x9a, 0x83, 0xc2, +0x20, 0xf6, 0xdb, 0x7d, 0x2e, 0xcb, 0x41, 0xed, +0xf2, 0x7c, 0x1e, 0x3e, 0x97, 0x41, 0x1b, 0xde, +0x06, 0x75, 0x4f, 0xd3, 0xd1, 0xa0, 0xf9, 0xbb, +0x5d, 0x06, 0x95, 0xf1, 0xf4, 0xb2, 0x9b, 0x5c, +0x26, 0x83, 0xc6, 0xe8, 0xb0, 0x3c, 0x3c, 0xa6, +0x97, 0xe5, 0xf3, 0x4d, 0x6a, 0xe3, 0x03, 0xd1, +0xd1, 0xf2, 0xf7, 0xdd, 0x0d, 0x4a, 0xbb, 0x59, +0xa0, 0x79, 0x7d, 0x8c, 0x06, 0x85, 0xe7, 0xa0, +0x00, 0x28, 0xec, 0x26, 0x83, 0x2a, 0x96, 0x0b, +0x28, 0xec, 0x26, 0x83, 0xbe, 0x60, 0x50, 0x78, +0x0e, 0xba, 0x97, 0xd9, 0x6c, 0x17}; + GHashTable *sms; + GError *error; + char *hexpdu; + + hexpdu = utils_bin2hexstr (pdu, sizeof(pdu)); + sms = sms_parse_pdu (hexpdu, error); + g_assert (sms); + + TEST_ENTRY_EQ (sms, smsc, +12404492164); + TEST_ENTRY_EQ (sms, number, +16175927198); + TEST_ENTRY_EQ (sms, timestamp, 110228115050-05); + TEST_ENTRY_EQ (sms, text, +Here's a longer message [{with some extended characters}] +thrown in, such as £ and ΩΠΨ and §¿ as well.); + + g_free (hexpdu); + g_hash_table_unref (sms); +} + +static void +test_pdu2 (void *f, gpointer d) +{ + static const guint8 pdu[] = { +0x07, 0x91, 0x97, 0x30, 0x07, 0x11, 0x11, 0xf1, +0x04, 0x14, 0xd0, 0x49, 0x37, 0xbd, 0x2c, 0x77, +0x97, 0xe9, 0xd3, 0xe6, 0x14, 0x00, 0x08, 0x11, +0x30, 0x92, 0x91, 0x02, 0x40, 0x61, 0x08, 0x04, +0x42, 0x04, 0x35, 0x04, 0x41, 0x04, 0x42}; + GHashTable *sms; + GError *error; + char *hexpdu; + + hexpdu = utils_bin2hexstr (pdu, sizeof(pdu)); + sms = sms_parse_pdu (hexpdu, error); + g_assert (sms); + + TEST_ENTRY_EQ (sms, smsc, +7903701); + TEST_ENTRY_EQ (sms, number, InternetSMS); + TEST_ENTRY_EQ (sms, timestamp, 110329192004+04); + TEST_ENTRY_EQ (sms, text, ÑеÑÑ); + + g_free (hexpdu); + g_hash_table_unref (sms); +} + +static void +test_pdu3 (void *f, gpointer d) +{ + static const guint8 pdu[] = { +0x07, 0x91
[ModemManager] [PATCH 3/4] Recognize more text encodings from GSM-03.38.
Third in a series. This fixes the bug detected by the dcsf1 test in the just-added unit tests, by more thoroughly parsing the TP-DCS field. - Nathan From a64fe628ac4f620878611f93f89deebe90fbb638 Mon Sep 17 00:00:00 2001 From: Nathan Williams n...@chromium.org Date: Wed, 29 Jun 2011 21:29:47 -0400 Subject: [PATCH 3/4] Recognize more text encodings from GSM-03.38. When parsing the DCS (data-coding scheme) field of a SMS message, recognize coding group F and the message-waiting coding groups. Presume GSM7 for otherwise-reserved encodings, as required. Change-Id: I0f808cbfbddc1c331bbd22b8cecce1cf94e05e96 --- src/mm-sms-utils.c | 89 +--- 1 files changed, 77 insertions(+), 12 deletions(-) diff --git a/src/mm-sms-utils.c b/src/mm-sms-utils.c index c7de0e1..6b01cf5 100644 --- a/src/mm-sms-utils.c +++ b/src/mm-sms-utils.c @@ -49,6 +49,13 @@ #define SMS_TIMESTAMP_LEN 7 #define SMS_MIN_PDU_LEN (7 + SMS_TIMESTAMP_LEN) +typedef enum { +MM_SMS_ENCODING_UNKNOWN = 0x0, +MM_SMS_ENCODING_GSM7, +MM_SMS_ENCODING_8BIT, +MM_SMS_ENCODING_UCS2 +} SmsEncoding; + static char sms_bcd_chars[] = 0123456789*#abc\0\0; static void @@ -122,21 +129,78 @@ sms_decode_timestamp (const guint8 *timestamp) return timestr; } +static SmsEncoding +sms_encoding_type (int dcs) +{ +SmsEncoding scheme = MM_SMS_ENCODING_UNKNOWN; + +switch ((dcs 4) 0xf) { +/* General data coding group */ +case 0: case 1: +case 2: case 3: +switch (dcs 0x0c) { +case 0x08: +scheme = MM_SMS_ENCODING_UCS2; +break; +case 0x00: +/* fallthrough */ +/* reserved - spec says to treat it as default alphabet */ +case 0x0c: +scheme = MM_SMS_ENCODING_GSM7; +break; +case 0x04: +scheme = MM_SMS_ENCODING_8BIT; +break; +} +break; + +/* Message waiting group (default alphabet) */ +case 0xc: +case 0xd: +scheme = MM_SMS_ENCODING_GSM7; +break; + +/* Message waiting group (UCS2 alphabet) */ +case 0xe: +scheme = MM_SMS_ENCODING_UCS2; +break; + +/* Data coding/message class group */ +case 0xf: +switch (dcs 0x04) { +case 0x00: +scheme = MM_SMS_ENCODING_GSM7; +break; +case 0x04: +scheme = MM_SMS_ENCODING_8BIT; +break; +} +break; + +/* Reserved coding group values - spec says to treat it as default alphabet */ +default: +scheme = MM_SMS_ENCODING_GSM7; +break; +} + +return scheme; + +} + static char * -sms_decode_text (const guint8 *text, int len, int dcs, int bit_offset) +sms_decode_text (const guint8 *text, int len, SmsEncoding encoding, int bit_offset) { char *utf8; -guint8 coding = dcs SMS_DCS_CODING_MASK; guint8 *unpacked; guint32 unpacked_len; -if (coding == SMS_DCS_CODING_DEFAULT) { +if (encoding == MM_SMS_ENCODING_GSM7) { unpacked = gsm_unpack ((const guint8 *) text, len, bit_offset, unpacked_len); utf8 = (char *) mm_charset_gsm_unpacked_to_utf8 (unpacked, unpacked_len); g_free (unpacked); -} else if (coding == SMS_DCS_CODING_UCS2) +} else if (encoding == MM_SMS_ENCODING_UCS2) utf8 = g_convert ((char *) text, len, UTF8, UCS-2BE, NULL, NULL, NULL); -else if (coding == SMS_DCS_CODING_8BIT) +else if (encoding == MM_SMS_ENCODING_8BIT) utf8 = g_strndup ((const char *)text, len); else utf8 = g_strdup (); @@ -198,8 +262,9 @@ sms_parse_pdu (const char *hexpdu, GError **error) int smsc_addr_num_octets, variable_length_items, msg_start_offset, sender_addr_num_digits, sender_addr_num_octets, tp_pid_offset, tp_dcs_offset, user_data_offset, user_data_len, -user_data_len_offset, user_data_dcs, bit_offset; +user_data_len_offset, bit_offset; char *smsc_addr, *sender_addr, *sc_timestamp, *msg_text; +SmsEncoding user_data_encoding; /* Convert PDU from hex to binary */ pdu = (guint8 *) utils_hexstr2bin (hexpdu, pdu_len); @@ -246,8 +311,8 @@ sms_parse_pdu (const char *hexpdu, GError **error) user_data_len_offset = tp_dcs_offset + 1 + SMS_TIMESTAMP_LEN; user_data_offset = user_data_len_offset + 1; user_data_len = pdu[user_data_len_offset]; -user_data_dcs = pdu[tp_dcs_offset]; -if ((user_data_dcs SMS_DCS_CODING_MASK) == SMS_DCS_CODING_DEFAULT) +user_data_encoding = sms_encoding_type(pdu[tp_dcs_offset]); +if (user_data_encoding == MM_SMS_ENCODING_GSM7) variable_length_items += (7 * (user_data_len + 1
[ModemManager] [PATCH 4/4] Calculate user-data bit padding correctly.
Fourth and final in a series. This fixes an off-by-one (septet) error in the calculation of the amount of data to skip in the presence of a user data header, and adds the test case from the wild that triggered it. - Nathan From 2c7a7c5752d42b7c2a1bb75865889eb49aaeeb30 Mon Sep 17 00:00:00 2001 From: Nathan Williams n...@chromium.org Date: Wed, 29 Jun 2011 21:33:37 -0400 Subject: [PATCH 4/4] Calculate user-data bit padding correctly. Deal correctly with the bit padding for the user data header for the case where the correct amount of padding is 0. Prevents skipping the first character of the message. Add a unit test for this. Change-Id: Ifa7bf7ea173b7da7aecffb9f4285ef2d8467cb03 --- src/mm-sms-utils.c |6 +- src/tests/test-sms.c | 29 + 2 files changed, 34 insertions(+), 1 deletions(-) diff --git a/src/mm-sms-utils.c b/src/mm-sms-utils.c index 6b01cf5..3f56a64 100644 --- a/src/mm-sms-utils.c +++ b/src/mm-sms-utils.c @@ -349,7 +349,11 @@ sms_parse_pdu (const char *hexpdu, GError **error) udhl = pdu[user_data_offset] + 1; user_data_offset += udhl; if (user_data_encoding == MM_SMS_ENCODING_GSM7) { -bit_offset = 7 - (udhl * 8) % 7; +/* + * Find the number of bits we need to add to the length of the + * user data to get a multiple of 7 (the padding). + */ +bit_offset = (7 - udhl % 7) % 7; user_data_len -= (udhl * 8 + bit_offset) / 7; } else user_data_len -= udhl; diff --git a/src/tests/test-sms.c b/src/tests/test-sms.c index dfbb195..bd18c0b 100644 --- a/src/tests/test-sms.c +++ b/src/tests/test-sms.c @@ -339,6 +339,34 @@ test_pdu_insufficient_data (void *f, gpointer d) g_free (hexpdu); } + +static void +test_pdu_udhi (void *f, gpointer d) +{ + /* Welcome message from KPN NL */ + static const char *hexpdu = +07911356131313F64004850120390011609232239180A006080400100201D7327BFD6EB340E232 +1BF46E83EA7790F59D1E97DBE1341B442F83C465763D3DA797E56537C81D0ECB41AB59CC1693C1 +6031D96C064241E5656838AF03A96230982A269BCD462917C8FA4E8FCBED709A0D7ABBE9F6B0FB +5C7683D27350984D4FABC9A0B33C4C4FCF5D20EBFB2D079DCB62793DBD06D9C36E50FB2D4E97D9 +A0B49B5E96BBCB; + GHashTable *sms; + GError *error; + + sms = sms_parse_pdu (hexpdu, error); + g_assert (sms); + + TEST_ENTRY_EQ (sms, smsc, +31653131316); + TEST_ENTRY_EQ (sms, number, 1002); + TEST_ENTRY_EQ (sms, timestamp, 110629233219+02); + TEST_ENTRY_EQ (sms, text, + Welkom, bel om uw Voicemail te beluisteren naar +31612001233 + (PrePay: *100*1233#). Voicemail ontvangen is altijd gratis. + Voor gebruik van mobiel interne); + + g_hash_table_unref (sms); +} + #if 0 static void test_pduX (void *f, gpointer d) @@ -393,6 +421,7 @@ int main (int argc, char **argv) g_test_suite_add (suite, TESTCASE (test_pdu_dcsf1, NULL)); g_test_suite_add (suite, TESTCASE (test_pdu_dcsf_8bit, NULL)); g_test_suite_add (suite, TESTCASE (test_pdu_insufficient_data, NULL)); +g_test_suite_add (suite, TESTCASE (test_pdu_udhi, NULL)); result = g_test_run (); -- 1.7.3.1 ___ networkmanager-list mailing list networkmanager-list@gnome.org http://mail.gnome.org/mailman/listinfo/networkmanager-list
[ModemManager] PATCH: Fix a corner case of SPN handling
This fixes a glitch where we return NULL (and log an assertion failure) rather than returning for a fully-empty SPN string, as seen on some European SIM cards (most recently tested with a Vodafone UK SIM). - Nathan 0002-get_spn_done-Handle-the-case-of-an-entirely-empty-SP.patch Description: Binary data ___ networkmanager-list mailing list networkmanager-list@gnome.org http://mail.gnome.org/mailman/listinfo/networkmanager-list
[ModemManager] PATCH: Add a DBus interface for setting the log level
Pretty straightforward, and as the patch comment says, lifted almost directly from the NetworkManager interface and code to do the same job. - Nathan 0001-Add-a-DBus-interface-for-setting-the-log-level.patch Description: Binary data ___ networkmanager-list mailing list networkmanager-list@gnome.org http://mail.gnome.org/mailman/listinfo/networkmanager-list
[PATCH] ModemManager: handle change uevents as well as add
The udev strategy used by ModemManager, as expressed in 80-mm-candidate.rules, expects add events to be emitted at startup when device rules have been processed (said events usually being generated by udevadm trigger). However, since udev-152, udevadm trigger generates change events instead. It's possible for distros or startup scripts to work around this, but I suggest we should just go ahead and accept these events as-is. - Nathan From 4d51ee9ff7762d089fc85cb078c6a46bed0b6160 Mon Sep 17 00:00:00 2001 From: Nathan Williams n...@google.com Date: Wed, 8 Jun 2011 15:52:08 -0400 Subject: [PATCH] handle_uevent(): Handle change events as well as add, since that's what the udev replay gives us these days (as of udev-152). Change-Id: I5b194d72d8f46ac235748f183314a7c8e0393907 --- src/mm-manager.c |3 ++- 1 files changed, 2 insertions(+), 1 deletions(-) diff --git a/src/mm-manager.c b/src/mm-manager.c index 561d427..cde0747 100644 --- a/src/mm-manager.c +++ b/src/mm-manager.c @@ -909,7 +909,8 @@ handle_uevent (GUdevClient *client, /* We only care about tty/net devices when adding modem ports, * but for remove, also handle usb parent device remove events */ - if ((!strcmp (action, add) || !strcmp (action, move)) strcmp (subsys, usb) !=0 ) + if ((!strcmp (action, add) || !strcmp (action, move) !strcmp (action, change)) + strcmp (subsys, usb) !=0 ) device_added (self, device); else if (!strcmp (action, remove)) device_removed (self, device); -- 1.7.3.1 ___ networkmanager-list mailing list networkmanager-list@gnome.org http://mail.gnome.org/mailman/listinfo/networkmanager-list
[PATCH] ModemManager: Format SMS sender addresses with a + for international telephone numbers
This formats addresses in SMS messages (sender address and SMSC) in accordance with ITU E.164/E.123 by including a + before the number when it's an international phone number (according to the other address-type fields). - Nathan From 6b18bc94a9347972d0d180386ff92cac03a6bcbb Mon Sep 17 00:00:00 2001 From: Nathan Williams n...@google.com Date: Tue, 31 May 2011 13:50:17 -0400 Subject: [PATCH] sms_decode_address(): Add a leading + on international numbers. BUG=chromium-os-partner:4278 TEST=Send SMS from phone (to get +... format) and from AIM-SMS gateway (to get raw-digit format). Change-Id: I36eb9f1432a432435578180dfdb315b0e7ee5744 --- src/mm-generic-gsm.c | 28 1 files changed, 24 insertions(+), 4 deletions(-) diff --git a/src/mm-generic-gsm.c b/src/mm-generic-gsm.c index 9bc16c9..f7db4f2 100644 --- a/src/mm-generic-gsm.c +++ b/src/mm-generic-gsm.c @@ -3967,6 +3967,14 @@ mm_generic_gsm_get_charset (MMGenericGsm *self) #define SMS_TP_MTI_SMS_SUBMIT_REPORT 0x01 #define SMS_TP_MTI_SMS_STATUS_REPORT 0x02 +#define SMS_NUMBER_TYPE_MASK 0x70 +#define SMS_NUMBER_TYPE_UNKNOWN 0x00 +#define SMS_NUMBER_TYPE_INTL 0x10 +#define SMS_NUMBER_TYPE_ALPHA 0x50 + +#define SMS_NUMBER_PLAN_MASK 0x0f +#define SMS_NUMBER_PLAN_TELEPHONE 0x01 + #define SMS_TP_MMS0x04 #define SMS_TP_SRI0x20 #define SMS_TP_UDHI 0x40 @@ -4046,21 +4054,33 @@ sms_semi_octets_to_bcd_string (char *dest, const guint8 *octets, int num_octets) static char * sms_decode_address (const guint8 *address, int len) { -guint8 addrtype; +guint8 addrtype, addrplan; char *utf8; -addrtype = address[0]; +addrtype = address[0] SMS_NUMBER_TYPE_MASK; +addrplan = address[0] SMS_NUMBER_PLAN_MASK; address++; -if (addrtype == 0xd0) { +if (addrtype == SMS_NUMBER_TYPE_ALPHA) { guint8 *unpacked; guint32 unpacked_len; unpacked = gsm_unpack (address, (len * 4) / 7, 0, unpacked_len); utf8 = (char *)mm_charset_gsm_unpacked_to_utf8 (unpacked, unpacked_len); g_free(unpacked); +} else if (addrtype == SMS_NUMBER_TYPE_INTL + addrplan == SMS_NUMBER_PLAN_TELEPHONE) { +/* International telphone number, format as +1234567890 */ +utf8 = g_malloc (len + 3); /* '+' + digits + possible trailing 0xf + NUL */ +utf8[0] = '+'; +sms_semi_octets_to_bcd_string (utf8 + 1, address, (len + 1) / 2); } else { -utf8 = g_malloc (len + 2); /* may need one extra for trailing 0xf */ +/* + * All non-alphanumeric types and plans are just digits, but + * don't apply any special formatting if we don't know the + * format. + */ +utf8 = g_malloc (len + 2); /* digits + possible trailing 0xf + NUL */ sms_semi_octets_to_bcd_string (utf8, address, (len + 1) / 2); } -- 1.7.3.1 ___ networkmanager-list mailing list networkmanager-list@gnome.org http://mail.gnome.org/mailman/listinfo/networkmanager-list
[PATCH] ModemManager: Add and implement an interface to get a GSM SPN
This adds a feature in parallel with GetOperatorId, which gets the GSM Service Provider Name value from the SIM card; this is useful for displaying the name of the home operator when said operator is a MVNO or otherwise branded differently than the carrier's usual name. There's clearly some code to be factored out of the various routines that parse the response of AT+CRSM commands, but that cleanup will come in a separate patch. - Nathan From 94bb1798e85a9a5c91eeaa6b82c7300591953aa6 Mon Sep 17 00:00:00 2001 From: Nathan Williams n...@google.com Date: Fri, 20 May 2011 17:31:34 -0400 Subject: [PATCH] Spec out and implement a command to get a GSM SIM SPN value. Using a SIM with a SPN, run the following command: dbus-send --system --dest=org.freedesktop.ModemManager --print-reply /org/freedesktop/ModemManager/Modems/0 org.freedesktop.ModemManager.Modem.Gsm.Card.GetSpn Change-Id: I8f36c8432f40fa4e3cb3f8c6ceef16b2bdadf2a1 Reviewed-on: http://gerrit.chromium.org/gerrit/1464 Reviewed-by: Nathan J. Williams n...@chromium.org Tested-by: Nathan J. Williams n...@chromium.org --- ...org.freedesktop.ModemManager.Modem.Gsm.Card.xml | 13 +++ src/mm-generic-gsm.c | 94 src/mm-modem-gsm-card.c| 54 +++ src/mm-modem-gsm-card.h|8 ++ 4 files changed, 169 insertions(+), 0 deletions(-) diff --git a/introspection/org.freedesktop.ModemManager.Modem.Gsm.Card.xml b/introspection/org.freedesktop.ModemManager.Modem.Gsm.Card.xml index bf33f4c..e6af331 100644 --- a/introspection/org.freedesktop.ModemManager.Modem.Gsm.Card.xml +++ b/introspection/org.freedesktop.ModemManager.Modem.Gsm.Card.xml @@ -42,6 +42,19 @@ /arg /method +method name=GetSpn + tp:docstring + Returns the SPN (Service Provider Name) from the SIM card, + /tp:docstring + annotation name=org.freedesktop.DBus.GLib.Async value=/ + annotation name=org.freedesktop.DBus.GLib.CSymbol value=impl_gsm_modem_get_spn/ + arg name=spn type=s direction=out + tp:docstring + The Service Provider Name. + /tp:docstring + /arg +/method + method name=SendPuk tp:docstring Send the PUK and a new PIN to unlock the SIM card. diff --git a/src/mm-generic-gsm.c b/src/mm-generic-gsm.c index 58454da..5b33ed7 100644 --- a/src/mm-generic-gsm.c +++ b/src/mm-generic-gsm.c @@ -1847,6 +1847,81 @@ get_operator_id_imsi_done (MMModem *modem, } static void +get_spn_done (MMAtSerialPort *port, + GString *response, + GError *error, + gpointer user_data) +{ +MMCallbackInfo *info = (MMCallbackInfo *) user_data; +int sw1, sw2; +gboolean success = FALSE; +char hex[51]; +char *bin, *utf8; + +if (error) { +info-error = g_error_copy (error); +goto done; +} + +memset (hex, 0, sizeof (hex)); +if (sscanf (response-str, +CRSM:%d,%d,\%50c\, sw1, sw2, (char *) hex) == 3) +success = TRUE; +else { +/* May not include quotes... */ +if (sscanf (response-str, +CRSM:%d,%d,%50c, sw1, sw2, (char *) hex) == 3) +success = TRUE; +} + +if (!success) { +info-error = g_error_new_literal (MM_MODEM_ERROR, + MM_MODEM_ERROR_GENERAL, + Could not parse the CRSM response); +goto done; +} + +if ((sw1 == 0x90 sw2 == 0x00) || (sw1 == 0x91) || (sw1 == 0x92) || (sw1 == 0x9f)) { +gsize buflen = 0; + +/* Make sure the buffer is only hex characters */ +while (buflen sizeof (hex) hex[buflen]) { +if (!isxdigit (hex[buflen])) { +hex[buflen] = 0x0; +break; +} +buflen++; +} + +/* Convert hex string to binary */ +bin = utils_hexstr2bin (hex, buflen); +if (!bin) { +info-error = g_error_new (MM_MODEM_ERROR, + MM_MODEM_ERROR_GENERAL, + SIM returned malformed response '%s', + hex); +goto done; +} + +/* Remove the FF filler at the end */ +while (bin[buflen - 1] == (char)0xff) +buflen--; + +/* First byte is metadata; remainder is GSM-7 unpacked into octets; convert to UTF8 */ +utf8 = (char *)mm_charset_gsm_unpacked_to_utf8 ((guint8 *)bin + 1, buflen - 1); +g_free(bin); +mm_callback_info_set_result(info, utf8, g_free); +} else { +info-error = g_error_new (MM_MODEM_ERROR, + MM_MODEM_ERROR_GENERAL, + SIM failed to handle CRSM request (sw1 %d sw2 %d), + sw1, sw2); +} + +done: +mm_callback_info_schedule (info); +} + + +static void get_imei (MMModemGsmCard *modem
[PATCH] ModemManager: Free an allocated string
Just like it says. - Nathan From 97a4b734229a6a9a86f4f819d6c79567c743baaa Mon Sep 17 00:00:00 2001 From: Nathan Williams n...@google.com Date: Fri, 20 May 2011 14:42:16 -0400 Subject: [PATCH] gsm: free the string allocated by utils_hexstr2bin(). Change-Id: I1f7dabc8209d9757b573a59abb788a2346f72ad5 --- src/mm-generic-gsm.c |2 ++ 1 files changed, 2 insertions(+), 0 deletions(-) diff --git a/src/mm-generic-gsm.c b/src/mm-generic-gsm.c index 0a0a3c1..398efc1 100644 --- a/src/mm-generic-gsm.c +++ b/src/mm-generic-gsm.c @@ -1801,6 +1801,7 @@ get_mnc_length_done (MMAtSerialPort *port, MM_MODEM_ERROR_GENERAL, SIM returned malformed response '%s', hex); +g_free (bin); goto done; } @@ -1815,6 +1816,7 @@ get_mnc_length_done (MMAtSerialPort *port, SIM returned invalid MNC length %d (should be either 2 or 3), mnc_len); } +g_free (bin); } else { info-error = g_error_new (MM_MODEM_ERROR, MM_MODEM_ERROR_GENERAL, -- 1.7.3.1 ___ networkmanager-list mailing list networkmanager-list@gnome.org http://mail.gnome.org/mailman/listinfo/networkmanager-list
[PATCH] ModemManager: gsm: Correctly set registration status when disabling
Pretty much like it says; without this, the circuit-switched status isn't set back to UNKNOWN at disable time, which messes with the value returned by gsm_reg_status(). Pesky integer types all being the same. - Nathan From ea36f0d7554a4c6a6ce3e9bb487c4c687cbd9798 Mon Sep 17 00:00:00 2001 From: Nathan Williams n...@chromium.org Date: Wed, 18 May 2011 20:37:49 -0400 Subject: [PATCH] gsm: Correctly set registration status when disabling. Change-Id: I0629706985f273832ac3662acb260388d0e6ed83 --- src/mm-generic-gsm.c |6 ++ 1 files changed, 2 insertions(+), 4 deletions(-) diff --git a/src/mm-generic-gsm.c b/src/mm-generic-gsm.c index 58454da..1fa7235 100644 --- a/src/mm-generic-gsm.c +++ b/src/mm-generic-gsm.c @@ -1594,14 +1594,12 @@ disable_done (MMAtSerialPort *port, /* Clear out circuit-switched registration info... */ reg_info_updated (self, - MM_GENERIC_GSM_REG_TYPE_CS, - TRUE, MM_MODEM_GSM_NETWORK_REG_STATUS_UNKNOWN, + TRUE, MM_GENERIC_GSM_REG_TYPE_CS, MM_MODEM_GSM_NETWORK_REG_STATUS_UNKNOWN, TRUE, NULL, TRUE, NULL); /* ...and packet-switched registration info */ reg_info_updated (self, - MM_GENERIC_GSM_REG_TYPE_PS, - TRUE, MM_MODEM_GSM_NETWORK_REG_STATUS_UNKNOWN, + TRUE, MM_GENERIC_GSM_REG_TYPE_PS, MM_MODEM_GSM_NETWORK_REG_STATUS_UNKNOWN, TRUE, NULL, TRUE, NULL); } -- 1.7.3.1 ___ networkmanager-list mailing list networkmanager-list@gnome.org http://mail.gnome.org/mailman/listinfo/networkmanager-list
[PATCH] ModemManager: Split the Samsung initialization sequence so that echoing is properly disabled
We were sometimes seeing the IMSI reported as AT+CIMI\n\r\n\r31026... because echoing was not always turned off at the point that the IMSI is requested. This patch fixes that by splitting the reset commands so that the second part of the command gets processed after the reset is complete. (This wasn't always a problem because the initial IMSI request usually fails since the SIM is not yet powered up. With the right enable/disable sequences, however, the initial AT+CIMI would succeed, and be echoed into the returned value.) - Nathan From fa69fc6d4019b909db77e387ab5c9a5765c85159 Mon Sep 17 00:00:00 2001 From: Nathan Williams n...@chromium.org Date: Thu, 12 May 2011 15:23:33 -0400 Subject: [PATCH] Split the Samsung initialization sequence from ATZ E0 V1 to ATZ and ATE0 V1 - the modem is allowed to ignore the rest of the line after Z, so echoing was not being turned off, leading to getting AT+CIMI\n\n as part of the IMSI when it is retrieved at startup. Change-Id: Icfd767174e779e472f8cde419acb163128e4715d --- plugins/mm-modem-samsung-gsm.c | 17 - 1 files changed, 16 insertions(+), 1 deletions(-) diff --git a/plugins/mm-modem-samsung-gsm.c b/plugins/mm-modem-samsung-gsm.c index 8b51ca8..6ee2e16 100755 --- a/plugins/mm-modem-samsung-gsm.c +++ b/plugins/mm-modem-samsung-gsm.c @@ -704,6 +704,21 @@ init_done (MMAtSerialPort *port, } static void +init_reset_done (MMAtSerialPort *port, + GString *response, + GError *error, + gpointer user_data) +{ +MMCallbackInfo *info = (MMCallbackInfo *) user_data; +MMModemSamsungGsm *self = MM_MODEM_SAMSUNG_GSM (info-modem); + +if (error) +mm_generic_gsm_enable_complete (MM_GENERIC_GSM (self), error, info); +else +mm_at_serial_port_queue_command (port, E0 V1, 3, init_done, info); +} + +static void do_enable (MMGenericGsm *modem, MMModemFn callback, gpointer user_data) { MMCallbackInfo *info; @@ -713,7 +728,7 @@ do_enable (MMGenericGsm *modem, MMModemFn callback, gpointer user_data) primary = mm_generic_gsm_get_at_port (modem, MM_PORT_TYPE_PRIMARY); g_assert (primary); -mm_at_serial_port_queue_command (primary, Z E0 V1, 3, init_done, info); +mm_at_serial_port_queue_command (primary, Z, 3, init_reset_done, info); } static void -- 1.7.3.1 ___ networkmanager-list mailing list networkmanager-list@gnome.org http://mail.gnome.org/mailman/listinfo/networkmanager-list
[PATCH] Suppress duplicate SMS notifications
On some modems, such as the Samsung Y3300, unsolicited notifications for SMS messages (+CMTI) are sent over both the primary and secondary ports. Currently, this leads to multiple SmsReceived signals being sent. This patch de-dups the signals by tracking the index numbers that have been seen so far. The data structure here could also be a basis for coalescing multipart messages. - Nathan --- src/mm-generic-gsm.c | 15 +++ 1 files changed, 15 insertions(+), 0 deletions(-) diff --git a/src/mm-generic-gsm.c b/src/mm-generic-gsm.c index 58454da..3340672 100644 --- a/src/mm-generic-gsm.c +++ b/src/mm-generic-gsm.c @@ -121,6 +121,9 @@ typedef struct { gboolean loc_signal; MMModemGsmUssdState ussd_state; + +/* SMS */ +GHashTable *sms_present; } MMGenericGsmPrivate; static void get_registration_status (MMAtSerialPort *port, MMCallbackInfo *info); @@ -1312,6 +1315,7 @@ cmti_received (MMAtSerialPort *port, gpointer user_data) { MMGenericGsm *self = MM_GENERIC_GSM (user_data); +MMGenericGsmPrivate *priv = MM_GENERIC_GSM_GET_PRIVATE (self); guint idx=0; char *mem; @@ -1324,6 +1328,13 @@ cmti_received (MMAtSerialPort *port, idx = atoi (str); g_free (str); +/* Don't signal multiple times if there are multiple CMTI notifications for a message */ +if (g_hash_table_lookup_extended (priv-sms_present, (void *)idx, NULL, NULL)) +return; + +/* Nothing is currently stored in the hash table - presence is all that matters. */ +g_hash_table_insert (priv-sms_present, (void *)idx, NULL); + /* todo: parse pdu to know if the sms is complete */ mm_modem_gsm_sms_received (MM_MODEM_GSM_SMS (self), idx, @@ -4227,8 +4238,10 @@ sms_delete (MMModemGsmSms *modem, MMCallbackInfo *info; char *command; MMAtSerialPort *port; +MMGenericGsmPrivate *priv = MM_GENERIC_GSM_GET_PRIVATE (MM_GENERIC_GSM (modem)); info = mm_callback_info_new (MM_MODEM (modem), callback, user_data); +g_hash_table_remove (priv-sms_present, (void *)idx); port = mm_generic_gsm_get_best_at_port (MM_GENERIC_GSM (modem), info-error); if (!port) { @@ -5237,6 +5250,7 @@ mm_generic_gsm_init (MMGenericGsm *self) priv-act = MM_MODEM_GSM_ACCESS_TECH_UNKNOWN; priv-reg_regex = mm_gsm_creg_regex_get (TRUE); priv-roam_allowed = TRUE; +priv-sms_present = g_hash_table_new (g_direct_hash, g_direct_equal); mm_properties_changed_signal_register_property (G_OBJECT (self), MM_MODEM_GSM_NETWORK_ALLOWED_MODE, @@ -5454,6 +5468,7 @@ finalize (GObject *object) g_free (priv-oper_code); g_free (priv-oper_name); g_free (priv-simid); +g_hash_table_destroy (priv-sms_present); G_OBJECT_CLASS (mm_generic_gsm_parent_class)-finalize (object); } -- 1.7.3.1 ___ networkmanager-list mailing list networkmanager-list@gnome.org http://mail.gnome.org/mailman/listinfo/networkmanager-list
[PATCH] GSM SMS reception code
Here's the implementation I've done of SMS reception code for GSM modems, exclusive of the bug fixes discussed here earlier. It implements the SmsReceived signal and the Get, Delete, and List commands; it does not group multi-part messages together into one, but does skip over the user-data header to avoid presenting garbage to the user. Credit to Torgny Johansson torgny.johans...@ericsson.com for the initial basis of this code. - Nathan diff --git a/introspection/org.freedesktop.ModemManager.Modem.Gsm.SMS.xml b/introspection/org.freedesktop.ModemManager.Modem.Gsm.SMS.xml index 081ecc5..15953b8 100644 --- a/introspection/org.freedesktop.ModemManager.Modem.Gsm.SMS.xml +++ b/introspection/org.freedesktop.ModemManager.Modem.Gsm.SMS.xml @@ -36,6 +36,7 @@ validity : uint (0..255) - Specifies when the SMS expires in SMSC (optional) class: uint (0..3) - Message importance and location (optional) completed: boolean - Whether all message parts have been received or not (optional) + index: uint - Index of message (for Get and Delete) (optional) /tp:docstring /arg /method diff --git a/src/mm-charsets.c b/src/mm-charsets.c index dd1ae08..cbdf388 100644 --- a/src/mm-charsets.c +++ b/src/mm-charsets.c @@ -427,16 +427,16 @@ mm_charset_utf8_to_unpacked_gsm (const char *utf8, guint32 *out_len) guint8 * gsm_unpack (const guint8 *gsm, -guint32 nchars, +guint32 num_septets, guint8 start_offset, /* in _bits_ */ guint32 *out_unpacked_len) { GByteArray *unpacked; int i; -unpacked = g_byte_array_sized_new (nchars + 1); +unpacked = g_byte_array_sized_new (num_septets + 1); -for (i = 0; i nchars; i++) { +for (i = 0; i num_septets; i++) { guint8 bits_here, bits_in_next, octet, offset, c; guint32 start_bit; diff --git a/src/mm-charsets.h b/src/mm-charsets.h index efd89f3..50b0cce 100644 --- a/src/mm-charsets.h +++ b/src/mm-charsets.h @@ -53,7 +53,7 @@ guint8 *mm_charset_utf8_to_unpacked_gsm (const char *utf8, guint32 *out_len); guint8 *mm_charset_gsm_unpacked_to_utf8 (const guint8 *gsm, guint32 len); guint8 *gsm_unpack (const guint8 *gsm, -guint32 nchars, /* number of gsm characters, not octets */ +guint32 num_septets, guint8 start_offset, /* in bits */ guint32 *out_unpacked_len); diff --git a/src/mm-generic-gsm.c b/src/mm-generic-gsm.c index 98713b0..24c3d24 100644 --- a/src/mm-generic-gsm.c +++ b/src/mm-generic-gsm.c @@ -167,6 +167,16 @@ static void ciev_received (MMAtSerialPort *port, GMatchInfo *info, gpointer user_data); +static void cmti_received (MMAtSerialPort *port, + GMatchInfo *info, + gpointer user_data); + +#define GS_HASH_TAG get-sms +static GValue *simple_string_value (const char *str); +static GValue *simple_uint_value (guint32 i); +static GValue *simple_boolean_value (gboolean b); +static void simple_free_gvalue (gpointer data); + MMModem * mm_generic_gsm_new (const char *device, const char *driver, @@ -814,6 +824,9 @@ mm_generic_gsm_grab_port (MMGenericGsm *self, regex = g_regex_new (\\r\\n\\+CIEV: (\\d+),(\\d)\\r\\n, G_REGEX_RAW | G_REGEX_OPTIMIZE, 0, NULL); mm_at_serial_port_add_unsolicited_msg_handler (MM_AT_SERIAL_PORT (port), regex, ciev_received, self, NULL); + +regex = g_regex_new (\\r\\n\\+CMTI: \(\\S+)\,(\\d+)\\r\\n, G_REGEX_RAW | G_REGEX_OPTIMIZE, 0, NULL); +mm_at_serial_port_add_unsolicited_msg_handler (MM_AT_SERIAL_PORT (port), regex, cmti_received, self, NULL); g_regex_unref (regex); if (ptype == MM_PORT_TYPE_PRIMARY) { @@ -1293,6 +1306,32 @@ ciev_received (MMAtSerialPort *port, } static void +cmti_received (MMAtSerialPort *port, + GMatchInfo *info, + gpointer user_data) +{ +MMGenericGsm *self = MM_GENERIC_GSM (user_data); + +guint idx=0; +char *mem; +char *str; + +mem = g_match_info_fetch (info, 1); + +str = g_match_info_fetch (info, 2); +if (str) +idx = atoi (str); +g_free (str); + +/* todo: parse pdu to know if the sms is complete */ +mm_modem_gsm_sms_received (MM_MODEM_GSM_SMS (self), + idx, + TRUE); + +/* todo: send mm_modem_gsm_sms_completed if complete */ +} + +static void cmer_cb (MMAtSerialPort *port, GString *response, GError *error, @@ -1380,6 +1419,11 @@ mm_generic_gsm_enable_complete (MMGenericGsm *self, /* Try to enable XON/XOFF flow control */ mm_at_serial_port_queue_command (priv-primary, +IFC=1,1, 3, NULL, NULL); +/* Enable SMS notifications */ +mm_at_serial_port_queue_command (priv-primary, +CNMI=2,1,2,1,0, 3, NULL, NULL); +/* Set SMS storage
Re: SV: Ericsson Support for SMS receive
On Fri, Apr 8, 2011 at 2:57 AM, Torgny Johansson torgny.johans...@ericsson.com wrote: From: Dan Williams [mailto:d...@redhat.com] Sure, that sounds fine, unless we decide to just ignore SIM storage completely and use local storage like Marcel suggests. For me, just using ME would be fine. Nathan? ME-only works for me. I believe the original idea was that SmsReceived would be emitted to every unique SMS reported, no matter whether that SMS was complete or partial. Then, when the full SMS was received and reconstructed from all its parts, the Completed signal would be emitted. If the message was single-part, both SmsReceived *and* completed would be emitted for that message. Right, so for the final part (regardless if the sms consists of more than one message or not) you'll always send both? This was my interpretation of the API as well. However, I'm a little bit unclear on what the index numbers should be in the case of multipart messages. The different parts will have different low-level index numbers; should we be concealing those and only reporting up the index number of the first part we received? That is, does this seem like the correct sequence: Receive Part 1 of 3 as SMS index 5. Generate signal SmsReceived, index=5, complete=false. Receive Part 2 of 3 as SMS index 6. Generate signal SmsReceived, index=5, complete=false. Receive Part 3 of 3 as SMS index 7. Generate signal SmsReceived, index=5, complete=true; generate signal Completed, index=5, complete=true. (And if this is the case, do we respect Get or Delete operations on index 6 or 7? How about on index 5 before we're received all three parts?) If we wanted to avoid maintaining this index mapping, the alternative would seem to be to include the Part N of M metadata in the message dictionary and make it a problem for the client to solve. - Nathan ___ networkmanager-list mailing list networkmanager-list@gnome.org http://mail.gnome.org/mailman/listinfo/networkmanager-list
Re: SV: Ericsson Support for SMS receive
you do not have index numbers if these are forwarded to the ME right away. The index numbers are storage numbers on the SIM card. Or in the other memory spaces. So far we've been discussing (in the language of 3GPP TS 27.005) +CPMS=ME versus +CPMS=SM; both involve storage somewhere below ModemManager, index numbers, and get/delete. It sounds like you're introducing the distinction between +CNMI=1 and +CNMI=2 notifications. That might be useful, but it's an implementation detail. At any rate, it sounds like you're firmly in the camp that the numbers in the API should be synthetic rather than matching the low levels. - Nathan ___ networkmanager-list mailing list networkmanager-list@gnome.org http://mail.gnome.org/mailman/listinfo/networkmanager-list
Re: [PATCH] ModemManager: Change length input to gsm7 to take the number of characters rather than octets
Sure, updated patch attached. - Nathan On Thu, Apr 7, 2011 at 6:45 PM, Dan Williams d...@redhat.com wrote: On Thu, 2011-04-07 at 18:09 -0400, Nathan Williams wrote: As it says in the patch, this is necessary to distinguish between 7- and 8-character messages, since both are encoded in 7 octets. Can you fix up the testcases too? src/tests/test_unpack_gsm7: ** ERROR:test-charsets.c:103:test_unpack_gsm7: assertion failed (unpacked_len == sizeof (expected)): (11 == 12) Dan mm-patch-gsm7-2 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
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 d...@redhat.com 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
[PATCH] ModemManager: Change length input to gsm7 to take the number of characters rather than octets
As it says in the patch, this is necessary to distinguish between 7- and 8-character messages, since both are encoded in 7 octets. - Nathan mm-patch-gsm7 Description: Binary data ___ networkmanager-list mailing list networkmanager-list@gnome.org http://mail.gnome.org/mailman/listinfo/networkmanager-list
[PATCH] ModemManager: Don't destroy NULL hashes
This avoids annoying diagnostic messages from GLib, since some uses of SmsAuthInfo have NULL hash values and some do not. - Nathan mm-patch-no-hash-destroy Description: Binary data ___ 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