Re: Patch to nm-modem-probe

[Rick Jones <rick activeservice co uk>]

Dan

Here's another patch to nm-modem-probe, and to the .rules file that invokes it.

I've taken on some of your suggestions, but used the .rules file to identify the ZTE vid and pass extra arguments to nm-modem-probe. I've made CPMS an option not the default.

Sending the init string to the ZTE after only a 1-sec wait doesn't cause any problems, it just keeps responding ERROR until it's woken up enough to deal with it. I'd thought it caused other issues, but looks like I was wrong.

The only effective change in behaviour from the original with non-ZTE devices is 1-sec rather than .5-sec initial delay, and sending ATE0 initialisation, which should always be beneficial. The default timeout on that initialisation is only 2-secs so if the modem doesn't want to co-operate it won't hold things up.

Let me know if you're happy with this, or if you'd like me to do any more to it.

Cheers, Rick

--On Saturday, July 25, 2009 22:33:08 +0100 Rick Jones <rick activeservice co uk> wrote:

¦ Dan
¦ 
¦ Thanks for the feedback - see below ...
¦ 
¦ --On Thursday, July 23, 2009 15:02:56 -0400 Dan Williams <dcbw redhat com> wrote:
¦ ¦ --delay was originally meant to be an overall timeout, but I optimized
¦ ¦ that later on to fast-path modems that responded to something before
¦ ¦ that 5 seconds was up.  Many modems (especially built-in devices that
¦ ¦ boot long before udev starts up) will respond immediately and the hard 3
¦ ¦ second timeout was really unecessary.  Other modems do require that
¦ ¦ timeout.
¦ 
¦ ¦ Your patch will actually make all modems wait timeout_ms before being
¦ ¦ probed, so it does increase the probe latency quite a bit.  Additional
¦ ¦ to that, it adds a 1s latency right before sending the init command.
¦ 
¦ I take the point on built-in modems, but given all the other delays and waits involved in opening a connection, is 2 or 3 seconds actually significant? If this is happening at boot-up, surely it will all be finished by the time the user logs in, even with a few extra seconds.
¦ 
¦ ¦ What I'd recommend is the following:
¦ ¦ 
¦ ¦ 1) get rid of --initwait, and just use --delay.  After the initial 500ms
¦ ¦ wait that the original code had, send the init string every second until
¦ ¦ you get some response, or until the timeout expires.  That preserves the
¦ ¦ fastpath that built-in modems can take to get recognized much more
¦ ¦ quickly, but rightfully penalizes the ZTE device for being, well, dumb.
¦ 
¦ One problem here is that if you start sending an init string to the ZTE sooner than about 3 secs, it seems to do some strange things. I can't remember exactly what was going on, but that was one reason I wanted to be able to have a longer initial delay. I can get some log output from nm-modem-probe next week if you'd like - I don't have the time right now, sorry.
¦ 
¦ ¦ 2) get rid of --nocpms.  Lets use the --vid option to special-case ZTE
¦ ¦ and automatically determine what init string to send.
¦ 
¦ Are you suggesting hard-coding recognition of the ZTE vid into nm-modem-probe? My preference would be to put that kind of thing into the .rules file if necessary, and keep compiled code more generic. If it turns out that some other device can benefit from the CPMS tweak, or there are versions of the ZTE that object to it, it's much easer to fix a .rules file, especially by a user as a "get out of trouble" fix. That's why I made it a command line option (it could easily of course be --cpms so the default is not to send it).
¦ 
¦ It would mean updating the .rules file to have two branches, one for ZTE and one not (assuming you prefer to send +CPMS only to ZTE devices), but two branches might be necessary anyway if ZTEs need a longer timeout.
¦ 
¦ ¦ "there is a short window where the buffered data can be echoed back to
¦ ¦ the modem before this happens."
¦ ¦ 
¦ ¦ Could you describe that issue in a bit more detail?  I don't quite
¦ ¦ understand what's going on there.
¦ 
¦ I've had to infer quite a few things here, but I'm pretty sure of what is happening.
¦ 
¦ When I was first trying to use the device I would quite often get complete system hangs, requiring hard re-boot. When this happened it was usually immediately on clicking in the applet menu to make a connection (and other users have reported this too). I then started looking at the ttyUSB port modes using stty, and they seemed to be pretty random. In particular, crashes could be linked with echo mode being enabled. If I ran "stty raw -F /dev/ttyUSB2" after the modem was plugged in but before connecting, I never got any crashes. By running "stty echo ..." I could almost guarantee to crash. My first thought was that NM wasn't initialising the port properly, but looking at the code it clearly does.
¦ 
¦ IIRC, a port doesn't echo its input unless it is open, so prior to a connection being made, nothing bad is happening. But the moment it is open it will echo, and if there is buffered input to the port then this will immediately echo back out. I believe this will happen in the window between NM opening the port and setting the termio modes. So the combination of a stack of UMs (which the ZTE by default spits out) queued up in a USB buffer somewhere, the ttyUSB port suddenly echoing them all back, and the modem echoing them back again, etc.... creates a snarl up enough to hang the machine. I don't know if it's actually a crash/panic, or it simply does something like eat all the interrupts.
¦ 
¦ But the fact is that making sure the port is set to raw mode, and left that way, totally stops the problem. So the best thing is simply for nm-modem-probe not to reset the port after using it!
¦ 
¦ It doesn't serve any purpose anyway, because nothing is going to use the port in its original mode. I guess those kernel fixes might affect it, as it's clearly pretty marginal, as will turning of modem echo in nm-modem-probe, and stopping UMs. But I still think the port should be left in raw mode after nm-modem-probe has finished - belt & braces.
¦ 
¦ ¦ 
¦ ¦ Thanks for your work on ZTE devices!
¦ 
¦ No probs, I just want something that works. I'd probably have saved myself a lot of trouble if I'd gone with a carrier who supplies Icon :)
¦ 
¦ Cheers, Rick 








--- nm-modem-probe-orig.c	2009-07-06 15:36:36.000000000 +0100
+++ nm-modem-probe.c	2009-07-29 17:30:27.000000000 +0100
@@ -313,7 +313,7 @@
 	return x->tv_sec < y->tv_sec;
 }
 
-static int modem_probe_caps(int fd, glong timeout_ms)
+static int modem_probe_caps(int fd, glong timeout_ms, glong init_sec, const char* initstr)
 {
 	const char *gcap_responses[] = { GCAP_TAG, HUAWEI_EC121_TAG, NULL };
 	const char *terminators[] = { "OK", "ERROR", "ERR", "+CME ERROR", NULL };
@@ -323,14 +323,25 @@
 	GTimeVal start, end;
 	gboolean send_success;
 
-	/* If a delay was specified, start a bit later */
-	if (timeout_ms > 500) {
-		g_usleep (500000);
-		timeout_ms -= 500;
+	/* turn off echo and (optionally) send +CPMS to stop ZTE +ZUSIMR messages
+		tries at 1-sec intervals to max of init_sec secs.
+		(note this forces an initial delay of 1 sec)
+	*/
+	do {
+		g_usleep (1000000);
+		if (modem_send_command (fd, initstr)) {
+			const char *null_responses[] = { NULL };
+			/* wait 2 secs for reply, if we got a timeout take extra 2-secs off remaining time */
+			if (modem_wait_reply (fd, 2, null_responses, terminators, &term_idx, &reply) == -2)
+				init_sec -= 2;
+			--init_sec;
 	}
+		else
+			init_sec = 0;
+	} while (init_sec > 0 && term_idx != 0);
 
 	/* Standard response timeout case */
-	timeout_ms += 3000;
+	timeout_ms += 2000;
 
 	while (timeout_ms > 0) {
 		GTimeVal diff;
@@ -339,7 +350,7 @@
 		g_get_current_time (&start);
 
 		idx = term_idx = 0;
-		send_success = modem_send_command (fd, "AT+GCAP\r\n");
+		send_success = modem_send_command (fd, "AT+GCAP\r");
 		if (send_success)
 			idx = modem_wait_reply (fd, 2, gcap_responses, terminators, &term_idx, &reply);
 		else
@@ -391,7 +402,7 @@
 		reply = NULL;
 
 		verbose ("GCAP failed, trying ATI...");
-		if (modem_send_command (fd, "ATI\r\n")) {
+		if (modem_send_command (fd, "ATI\r")) {
 			idx = modem_wait_reply (fd, 3, ati_responses, terminators, &term_idx, &reply);
 			if (0 == term_idx && 0 == idx) {
 				verbose ("ATI response: %s", reply);
@@ -410,7 +421,7 @@
 	if ((idx != -2) && !(ret & MODEM_CAP_GSM) && !(ret & MODEM_CAP_IS707_A)) {
 		const char *cgmm_responses[] = { CGMM_TAG, NULL };
 
-		if (modem_send_command (fd, "AT+CGMM\r\n")) {
+		if (modem_send_command (fd, "AT+CGMM\r")) {
 			idx = modem_wait_reply (fd, 5, cgmm_responses, terminators, &term_idx, &reply);
 			if (0 == term_idx && 0 == idx) {
 				verbose ("CGMM response: %s", reply);
@@ -428,7 +439,9 @@
 {
 	printf("Usage: probe-modem [options] <device>\n"
 	    " --export               export key/value pairs\n"
-	    " --delay <ms>           delay before probing (1 to 3000 ms inclusive)\n"
+	    " --delay <ms>           delay before probing (1 to 5000 ms inclusive)\n"
+	    " --initwait             secs to wait for modem to initialise (optional, default 2)\n"
+	    " --cpms                 include +CPMS? in init string\n"
 	    " --verbose              print verbose debugging output\n"
 	    " --quiet                suppress logging to stdout (does not affect logfile output)\n"
 	    " --log <file>           log all output\n"
@@ -445,6 +458,8 @@
 	static const struct option options[] = {
 		{ "export", 0, NULL, 'x' },
 		{ "delay", required_argument, NULL, 'a' },
+		{ "initwait", required_argument, NULL, 'w' },
+		{ "cpms", 0, NULL, 'z' },
 		{ "verbose", 0, NULL, 'v' },
 		{ "quiet", 0, NULL, 'q' },
 		{ "log", required_argument, NULL, 'l' },
@@ -460,11 +475,12 @@
 	const char *logpath = NULL;
 	const char *driver = NULL;
 	gboolean export = 0;
-	struct termios orig, attrs;
+	struct termios attrs;
 	int fd = -1, caps, ret = 0;
-	guint32 delay_ms = 0;
+	guint32 delay_ms = 0, init_sec = 2;
 	unsigned int vid = 0, pid = 0, usbif = 0, last_err = 0;
 	unsigned long int tmp;
+	const char *initstr = "ATE0\r";
 
 	while (1) {
 		int option;
@@ -479,12 +495,23 @@
 			break;
 		case 'a':
 			tmp = strtoul (optarg, NULL, 10);
-			if (tmp < 1 || tmp > 3000) {
+			if (tmp < 1 || tmp > 5000) {
 				fprintf (stderr, "Invalid delay: %s\n", optarg);
 				return 1;
 			}
 			delay_ms = (guint32) tmp;
 			break;
+		case 'w':
+			tmp = strtoul (optarg, NULL, 10);
+			if (tmp < 1 || tmp > 15) {
+				fprintf (stderr, "Invalid initwait: %s\n", optarg);
+				return 1;
+			}
+			init_sec = (guint32) tmp;
+			break;
+		case 'z':		/* --cpms option - init string for ZTE devices */
+			initstr = "ATE0+CPMS?\r";
+			break;
 		case 'v':
 			verbose = TRUE;
 			break;
@@ -580,17 +607,15 @@
 		goto exit;
 	}
 
-	if (tcgetattr (fd, &orig)) {
+	if (tcgetattr (fd, &attrs)) {
 		g_printerr ("tcgetattr(%s): failed %d\n", device, errno);
 		ret = 5;
 		goto exit;
 	}
 
-	memcpy (&attrs, &orig, sizeof (attrs));
 	attrs.c_iflag &= ~(IGNCR | ICRNL | IUCLC | INPCK | IXON | IXANY | IGNPAR);
 	attrs.c_oflag &= ~(OPOST | OLCUC | OCRNL | ONLCR | ONLRET);
 	attrs.c_lflag &= ~(ICANON | XCASE | ECHO | ECHOE | ECHONL);
-	attrs.c_lflag &= ~(ECHO | ECHOE);
 	attrs.c_cc[VMIN] = 1;
 	attrs.c_cc[VTIME] = 0;
 	attrs.c_cc[VEOF] = 1;
@@ -599,8 +624,8 @@
 	attrs.c_cflag |= (B9600 | CS8 | CREAD | PARENB);

 	tcsetattr (fd, TCSANOW, &attrs);
-	caps = modem_probe_caps (fd, delay_ms);
-	tcsetattr (fd, TCSANOW, &orig);
+	caps = modem_probe_caps (fd, delay_ms, init_sec, initstr);
+	/* note: we deliberately don't restore the terminal attributes */
 
 	if (caps < 0) {
 		g_printerr ("%s: couldn't get modem capabilities\n", device);

--- 77-nm-probe-modem-capabilities.rules-orig	2009-07-06 15:36:36.000000000 +0100
+++ 77-nm-probe-modem-capabilities.rules	2009-07-29 17:40:09.000000000 +0100
@@ -17,6 +17,10 @@
 
 SUBSYSTEM=="tty", SUBSYSTEMS=="usb", DRIVERS=="?*", ENV{NM_MODEM_DRIVER}="$attr{driver}"
 SUBSYSTEM=="tty", SUBSYSTEMS=="usb", ATTRS{bInterfaceNumber}=="?*", ENV{NM_MODEM_USB_INTERFACE_NUMBER}="$attr{bInterfaceNumber}"
+
+# Special handling for ZTE modems
+SUBSYSTEM=="tty", SUBSYSTEMS=="usb", ATTRS{idVendor}=="19d2", ATTRS{idProduct}=="?*", IMPORT{program}="nm-modem-probe --vid 0x$attr{idVendor} --pid 0x$attr{idProduct} --usb-interface $env{NM_MODEM_USB_INTERFACE_NUMBER} --driver $env{NM_MODEM_DRIVER} --delay 3000 --cpms --initwait 10 --export $tempnode", GOTO="nm_modem_probe_end"
+
 SUBSYSTEM=="tty", SUBSYSTEMS=="usb", ATTRS{idVendor}=="?*", ATTRS{idProduct}=="?*", IMPORT{program}="nm-modem-probe --vid 0x$attr{idVendor} --pid 0x$attr{idProduct} --usb-interface $env{NM_MODEM_USB_INTERFACE_NUMBER} --driver $env{NM_MODEM_DRIVER} --delay 3000 --export $tempnode", GOTO="nm_modem_probe_end"
 
 LABEL="nm_modem_probe_end"