Re: [PATCH -v5 03/13] telit: add support the enable the SAP client modem

[Denis Kenzior <denkenz@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 3287 bytes --]

Hi Gustavo,

On 09/28/2011 04:24 PM, Gustavo F. Padovan wrote:
> From: "Gustavo F. Padovan" <padovan@profusion.mobi>
> 
> ---
>  plugins/bluetooth.h |    3 +-
>  plugins/telit.c     |  191 ++++++++++++++++++++++++++++++++++++++++++++++++---
>  2 files changed, 184 insertions(+), 10 deletions(-)

Patch has been applied, see below:

> +static int telit_sap_open()
> +{
> +	const char *device = "/dev/ttyUSB4";

This part should probably come from udev or a config file somewhere.
Worst case /etc/ofono/telitsap.conf or something like that.

> +	struct termios ti;
> +	int fd;
> +
> +	DBG("%s", device);
> +
> +	fd = open(device, O_RDWR | O_NOCTTY | O_NONBLOCK);
> +	if (fd < 0)
> +		return -EINVAL;
>  
> +	/* Switch TTY to raw mode */
> +	memset(&ti, 0, sizeof(ti));
> +	cfmakeraw(&ti);
> +
> +	ti.c_cflag |= (B115200 | CLOCAL | CREAD);
> +
> +	tcflush(fd, TCIOFLUSH);
> +	if (tcsetattr(fd, TCSANOW, &ti) < 0) {
> +		close(fd);
> +		return -EBADF;
> +	}
> +
> +	return fd;
>  }
>  
> -static int telit_sap_enable(struct ofono_modem *modem)
> +static int telit_sap_enable(struct ofono_modem *modem,
> +					struct ofono_modem *sap_modem,
> +					int bt_fd)
>  {
>  	struct telit_data *data = ofono_modem_get_data(modem);
> +	int fd;
>  
>  	DBG("%p", modem);
>  
> -	data->chat = open_device(modem, "Data", "Aux: ");
> -	if (data->chat == NULL)
> +	data->aux = open_device(modem, "Data", "Aux: ");
> +	if (data->aux == NULL)
>  		return -EINVAL;
>  
> -	g_at_chat_register(data->chat, "#RSEN:", telit_rsen_notify,
> +	fd = telit_sap_open();
> +	if (fd < 0)
> +		return fd;
> +
> +	data->hw_io = g_io_channel_unix_new(fd);
> +	if (data->hw_io == NULL) {
> +		close(fd);

You weren't closing the aux channel here

> +		return -ENOMEM;
> +	}
> +
> +	g_io_channel_set_encoding(data->hw_io, NULL, NULL);
> +	g_io_channel_set_buffered(data->hw_io, FALSE);
> +	g_io_channel_set_close_on_unref(data->hw_io, TRUE);
> +
> +	data->hw_watch = g_io_add_watch_full(data->hw_io, G_PRIORITY_DEFAULT,
> +				G_IO_HUP | G_IO_ERR | G_IO_NVAL | G_IO_IN,
> +				hw_event_cb, modem, hw_watch_remove);
> +
> +	data->bt_io = g_io_channel_unix_new(bt_fd);
> +	if (data->bt_io == NULL) {
> +		sap_close_io(modem);
> +		return -ENOMEM;
> +	}
> +
> +	g_io_channel_set_encoding(data->bt_io, NULL, NULL);
> +	g_io_channel_set_buffered(data->bt_io, FALSE);
> +	g_io_channel_set_close_on_unref(data->bt_io, TRUE);
> +
> +	data->bt_watch = g_io_add_watch_full(data->bt_io, G_PRIORITY_DEFAULT,
> +				G_IO_HUP | G_IO_ERR | G_IO_NVAL | G_IO_IN,
> +				bt_event_cb, modem, bt_watch_remove);
> +
> +
> +	data->sap_modem = sap_modem;
> +
> +	g_at_chat_register(data->aux, "#RSEN:", telit_rsen_notify,
>  				FALSE, modem, NULL);
>  
> -	g_at_chat_send(data->chat, "AT#NOPT=3", NULL, NULL, NULL, NULL);
> +	g_at_chat_send(data->aux, "AT#NOPT=0", NULL, NULL, NULL, NULL);
>  
>  	/* Set SAP functionality */
> -	g_at_chat_send(data->chat, "AT#RSEN=1,1,0,2,0", rsen_prefix,
> +	g_at_chat_send(data->aux, "AT#RSEN=1,1,0,2,0", rsen_prefix,
>  				rsen_enable_cb, modem, NULL);
>  
>  	return -EINPROGRESS;

So I reflowed this function in a follow on patch, please double check it
for correctness.

Regards,
-Denis