Skip to contentSkip to conversation

Added Wavecom modem plugin.

PATCH In review Matthias 2 participants 4 messages Apr 19, 2010 – Apr 20, 2010 Kbuild

lore
Share & tools
lore

Matthias

author
#0 lore raw

PATCH 1/1 Added Wavecom modem plugin.

The Wavecom WMP100 assumes CPIN to be the final response, therefore some special handling is neccessary (see 6d28f82dc1ccb3de3a028a88eafb96fb001c7e61). Now the vendor id OFONO_VENDOR_WAVECOM is passed with all "*create" functions.

---
 Makefile.am         |    3 +
 plugins/modemconf.c |    1 +
 plugins/wavecom.c   |  212 +++++++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 216 insertions(+), 0 deletions(-)
 create mode 100644 plugins/wavecom.c
Makefile.am +3 -0
index 0eaadda..9b4b31c 100644
@@ -187,6 +187,9 @@ builtin_sources += plugins/atgen.c
 builtin_modules += g1
 builtin_sources += plugins/g1.c
 
+builtin_modules += wavecom
+builtin_sources += plugins/wavecom.c
+
 builtin_modules += calypso
 builtin_sources += plugins/calypso.c
plugins/modemconf.c +1 -0
index cf0ee8b..f38ca98 100644
@@ -121,6 +121,7 @@ static struct {
 	{ "phonesim",	set_address	},
 	{ "atgen",	set_device	},
 	{ "g1",		set_device	},
+	{ "wavecom",	set_device	},
 	{ "ste",	set_device	},
 	{ "calypso",	set_device	},
 	{ "palmpre",	set_device	},
plugins/wavecom.c new file +212 -0

212 lines not on this page · raw message

Signature · 1 line
1.6.3.3
#1 lore raw

Hi Matthias,

Quoted 4 lines

The Wavecom WMP100 assumes CPIN to be the final response, therefore some
special handling is neccessary (see 6d28f82dc1ccb3de3a028a88eafb96fb001c7e61). Now the vendor id OFONO_VENDOR_WAVECOM is passed with all "*create" functions.

You should also set your authorship information correctly. E.g. first + last name.

Quoted 6 lines
plugins/wavecom.c new file +6 -0
+	options = g_hash_table_new_full(g_str_hash, g_str_equal,
+					g_free, g_free);
+	if (!options)
+		return -ENOMEM;
+
+	for (i = 0; tty_opts[i]; i++) {

plugins/wavecom.c line 110

Quoted 10 lines
plugins/wavecom.c new file +10 -0
+		value = ofono_modem_get_string(modem, tty_opts[i]);
+
+		if (value == NULL)
+			continue;
+
+		g_hash_table_insert(options, g_strdup(tty_opts[i]),
+					g_strdup(value));
+	}
+
+	channel = g_at_tty_open(device, options);

I suggest hardcoding these options (see calypso.c) unless you really want to change them from modem.conf for some reason.

plugins/wavecom.c line 128

Quoted 11 lines
plugins/wavecom.c new file +11 -0
+	value = ofono_modem_get_string(modem, "GsmSyntax");
+	if (value) {
+		if (g_str_equal(value, "V1"))
+			syntax = g_at_syntax_new_gsmv1();
+		else if (g_str_equal(value, "Permissive"))
+			syntax = g_at_syntax_new_gsm_permissive();
+		else
+			return -EINVAL;
+	} else {
+		syntax = g_at_syntax_new_gsmv1();
+	}

Same here, you should know whether the wavecom is 27.007 compliant or not.

plugins/wavecom.c line 166

Quoted 3 lines
plugins/wavecom.c new file +3 -0
+	ofono_devinfo_create(modem, OFONO_VENDOR_WAVECOM, "atmodem", chat);
+	ofono_sim_create(modem, OFONO_VENDOR_WAVECOM, "atmodem", chat);
+	ofono_voicecall_create(modem, OFONO_VENDOR_WAVECOM, "atmodem", chat);

You really don't want to do that, only pass it to the place where it is relevant (e.g. the SIM atom.) That way we know what quirks the modem actually has instead of looking at all drivers just to find it has completely stock behavior.

Regards,
-Denis

author
#2 lore raw

PATCH 1/1 Added Wavecom modem plugin.

The Wavecom WMP100 assumes CPIN to be the final response, therefore some special handling is neccessary (see 6d28f82dc1ccb3de3a028a88eafb96fb001c7e61). Now the vendor id OFONO_VENDOR_WAVECOM is passed with all "*create" functions.

---
 Makefile.am         |    3 +
 plugins/modemconf.c |    1 +
 plugins/wavecom.c   |  186 +++++++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 190 insertions(+), 0 deletions(-)
 create mode 100644 plugins/wavecom.c
Makefile.am +3 -0
index 4acc69b..3d078d8 100644
@@ -193,6 +193,9 @@ builtin_sources += plugins/atgen.c
 builtin_modules += g1
 builtin_sources += plugins/g1.c
 
+builtin_modules += wavecom
+builtin_sources += plugins/wavecom.c
+
 builtin_modules += calypso
 builtin_sources += plugins/calypso.c
plugins/modemconf.c +1 -0
index 281a762..3e6203a 100644
@@ -121,6 +121,7 @@ static struct {
 	{ "phonesim",	set_address	},
 	{ "atgen",	set_device	},
 	{ "g1",		set_device	},
+	{ "wavecom",	set_device	},
 	{ "ste",	set_device	},
 	{ "calypso",	set_device	},
 	{ "palmpre",	set_device	},
plugins/wavecom.c new file +186 -0

186 lines not on this page · raw message

Signature · 1 line
1.6.3.3
#3 lore raw

Hi Matthias,

Quoted 4 lines

The Wavecom WMP100 assumes CPIN to be the final response, therefore some
special handling is neccessary (see 6d28f82dc1ccb3de3a028a88eafb96fb001c7e61). Now the vendor id OFONO_VENDOR_WAVECOM is passed with all "*create" functions.

I massaged the commit message and pushed the patch. I did make two small changes afterward:

plugins/wavecom.c line 98

Quoted 3 lines
plugins/wavecom.c new file +3 -0
+	if (!channel) {
+		return -EIO;
+	}

Coding style requires this to be without braces (all single statement blocks if/while/for should not have braces)

plugins/wavecom.c line 102

Quoted 4 lines
plugins/wavecom.c new file +4 -0
+
+	/* Could not figure out whether it is fully compliant or not */
+	syntax = g_at_syntax_new_gsm_permissive();
+	//syntax = g_at_syntax_new_gsmv1();

Commented out code is not a good idea, if you're not sure about something reflect it in the comment.

Regards,
-Denis