[PATCH] When setting the serial port, the previous settings need to be preserved.

[PATCH] When setting the serial port, the previous settings need to be preserved.

[Ryan Raasch]

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

Hello,

This took a while to locate. Shouldn't it be possible to use stty
outside the daemon, and then daemon would just continue as usual?

In my case, the cread was being removed, so no input data could be read.

Thanks,
Ryan

Signed-off-by: Ryan M. Raasch <ryan.raasch@gmail.com>
---
 gatchat/gattty.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-When-setting-the-serial-port-the-previous-settings-n.patch --]
[-- Type: text/x-patch, Size: 328 bytes --]

diff --git a/gatchat/gattty.c b/gatchat/gattty.c
index 63d26d4..ed0fadf 100644
--- a/gatchat/gattty.c
+++ b/gatchat/gattty.c
@@ -220,6 +220,7 @@ static int open_device(const char *tty, GHashTable *options)
 	if (fd < 0)
 		return -1;
 
+	tcgetattr(fd,&ti);
 	tcflush(fd, TCIOFLUSH);
 	tcsetattr(fd, TCSANOW, &ti);
 

Re: [PATCH] When setting the serial port, the previous settings need to be preserved.

[Denis Kenzior]

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

Hi Ryan,

> This took a while to locate. Shouldn't it be possible to use stty
> outside the daemon, and then daemon would just continue as usual?
>
> In my case, the cread was being removed, so no input data could be read.

Sorry but the patch is wrong, since you're blowing away all options that have 
been set via the GHashTable.

GAtTty is really a convenience method, if you need something specific, pass the 
option in the GHashTable or setup the fd however you wish by yourself.

Regards,
-Denis

Re: [PATCH] When setting the serial port, the previous settings need to be preserved.

[Ryan Raasch]

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


Denis Kenzior wrote:
> Hi Ryan,
> 
>> This took a while to locate. Shouldn't it be possible to use stty
>> outside the daemon, and then daemon would just continue as usual?
>>
>> In my case, the cread was being removed, so no input data could be read.
> 
> Sorry but the patch is wrong, since you're blowing away all options that have 
> been set via the GHashTable.

Yea. Sorry. Put it in the wrong place, uggh. It should go before setting 
the options, since the switches are or'ing and xor'ing.

> 
> GAtTty is really a convenience method, if you need something specific, pass the 
> option in the GHashTable or setup the fd however you wish by yourself.
> 
> Regards,
> -Denis

But, there is a problem with the opening of the serial port b/c the 
CREAD flag is destroyed if it is active, making the serial port unreadable.

Ryan

> _______________________________________________
> ofono mailing list
> ofono(a)ofono.org
> http://lists.ofono.org/listinfo/ofono

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: serial_port_enable_CREAD.patch --]
[-- Type: text/x-patch, Size: 1410 bytes --]

 gatchat/gattty.c |   26 +++++++++++++++++++++-----
 1 files changed, 21 insertions(+), 5 deletions(-)

diff --git a/gatchat/gattty.c b/gatchat/gattty.c
index ed0fadf..aa9d3a4 100644
--- a/gatchat/gattty.c
+++ b/gatchat/gattty.c
@@ -97,6 +97,19 @@ static gboolean set_baud(const char *baud, struct termios *ti)
 	return TRUE;
 }
 
+static gboolean set_read(value, &ti)
+{
+	if (g_str_equal(bits, "off"))
+		ti->c_cflag &= ~(CREAD);
+	else if (g_str_equal(bits, "on"))
+		ti->c_cflag |= CREAD;
+	else
+		return FALSE;
+
+	return TRUE;
+
+}
+
 static gboolean set_stop_bits(const char *bits, struct termios *ti)
 {
 	if (g_str_equal(bits, "1"))
@@ -186,6 +199,12 @@ static int open_device(const char *tty, GHashTable *options)
 	memset(&ti, 0, sizeof(ti));
 	cfmakeraw(&ti);
 
+	fd = open(tty, O_RDWR | O_NOCTTY | O_NONBLOCK);
+	if (fd < 0)
+		return -1;
+
+	tcgetattr(fd,&ti);
+
 	if (options) {
 		GHashTableIter iter;
 		const char *key;
@@ -210,17 +229,14 @@ static int open_device(const char *tty, GHashTable *options)
 				ok = set_rtscts(value, &ti);
 			else if (g_str_equal(key, "local"))
 				ok = set_local(value, &ti);
+			else if (g_str_equal(key, "read"))
+				ok = set_read(value, &ti);
 
 			if (ok == FALSE)
 				return -1;
 		}
 	}
 
-	fd = open(tty, O_RDWR | O_NOCTTY | O_NONBLOCK);
-	if (fd < 0)
-		return -1;
-
-	tcgetattr(fd,&ti);
 	tcflush(fd, TCIOFLUSH);
 	tcsetattr(fd, TCSANOW, &ti);
 

Re: [PATCH] When setting the serial port, the previous settings need to be preserved.

[Ryan Raasch]

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

Sorry. I have had a long few days. The last patch was just COMPLETELY 
wrong :(

I think i got it right this time...

Apologies,

Ryan

Ryan Raasch wrote:
> 
> Denis Kenzior wrote:
>> Hi Ryan,
>>
>>> This took a while to locate. Shouldn't it be possible to use stty
>>> outside the daemon, and then daemon would just continue as usual?
>>>
>>> In my case, the cread was being removed, so no input data could be read.
>>
>> Sorry but the patch is wrong, since you're blowing away all options 
>> that have been set via the GHashTable.
> 
> Yea. Sorry. Put it in the wrong place, uggh. It should go before setting 
> the options, since the switches are or'ing and xor'ing.
> 
>>
>> GAtTty is really a convenience method, if you need something specific, 
>> pass the option in the GHashTable or setup the fd however you wish by 
>> yourself.
>>
>> Regards,
>> -Denis
> 
> But, there is a problem with the opening of the serial port b/c the 
> CREAD flag is destroyed if it is active, making the serial port unreadable.
> 
> Ryan
> 
>> _______________________________________________
>> ofono mailing list
>> ofono-bdc2hr5oBkPYtjvyW6yDsg(a)public.gmane.org
>> http://lists.ofono.org/listinfo/ofono
> 

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: serial_port_enable_CREAD.patch --]
[-- Type: text/x-patch, Size: 1311 bytes --]

diff --git a/gatchat/gattty.c b/gatchat/gattty.c
index 63d26d4..d624336 100644
--- a/gatchat/gattty.c
+++ b/gatchat/gattty.c
@@ -97,6 +97,19 @@ static gboolean set_baud(const char *baud, struct termios *ti)
 	return TRUE;
 }
 
+static gboolean set_read(const char *bits, struct termios *ti)
+{
+	if (g_str_equal(bits, "off"))
+		ti->c_cflag &= ~(CREAD);
+	else if (g_str_equal(bits, "on"))
+		ti->c_cflag |= CREAD;
+	else
+		return FALSE;
+
+	return TRUE;
+
+}
+
 static gboolean set_stop_bits(const char *bits, struct termios *ti)
 {
 	if (g_str_equal(bits, "1"))
@@ -186,6 +199,12 @@ static int open_device(const char *tty, GHashTable *options)
 	memset(&ti, 0, sizeof(ti));
 	cfmakeraw(&ti);
 
+	fd = open(tty, O_RDWR | O_NOCTTY | O_NONBLOCK);
+	if (fd < 0)
+		return -1;
+
+	tcgetattr(fd,&ti);
+
 	if (options) {
 		GHashTableIter iter;
 		const char *key;
@@ -210,16 +229,14 @@ static int open_device(const char *tty, GHashTable *options)
 				ok = set_rtscts(value, &ti);
 			else if (g_str_equal(key, "local"))
 				ok = set_local(value, &ti);
+			else if (g_str_equal(key, "read"))
+				ok = set_read(value, &ti);
 
 			if (ok == FALSE)
 				return -1;
 		}
 	}
 
-	fd = open(tty, O_RDWR | O_NOCTTY | O_NONBLOCK);
-	if (fd < 0)
-		return -1;
-
 	tcflush(fd, TCIOFLUSH);
 	tcsetattr(fd, TCSANOW, &ti);
 

Re: [PATCH] When setting the serial port, the previous settings need to be preserved.

[Denis Kenzior]

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

Hi Ryan,

> Sorry. I have had a long few days. The last patch was just COMPLETELY
> wrong :(
>
> I think i got it right this time...

So the patch is still wrong :)

 	memset(&ti, 0, sizeof(ti));
 	cfmakeraw(&ti);
 
+	fd = open(tty, O_RDWR | O_NOCTTY | O_NONBLOCK);
+	if (fd < 0)
+		return -1;
+
+	tcgetattr(fd,&ti);
+

You're blowing away the result of cfmakeraw.  Most of the devices are not true 
serial devices and so do not need any of the special options.  Passing a NULL 
options GHashTable should result in cfmakeraw.  Why don't you just pass all 
the options you need instead of using tcgetattr?

The other parts of the patch look good to me.

Regards,
-Denis

Re: [PATCH] When setting the serial port, the previous settings need to be preserved.

[Ryan Raasch]

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



Denis Kenzior wrote:
> Hi Ryan,
> 
>> Sorry. I have had a long few days. The last patch was just COMPLETELY
>> wrong :(
>>
>> I think i got it right this time...
> 
> So the patch is still wrong :)
> 
>  	memset(&ti, 0, sizeof(ti));
>  	cfmakeraw(&ti);
>  
> +	fd = open(tty, O_RDWR | O_NOCTTY | O_NONBLOCK);
> +	if (fd < 0)
> +		return -1;
> +
> +	tcgetattr(fd,&ti);
> +
> 
> You're blowing away the result of cfmakeraw.  Most of the devices are not true 
> serial devices and so do not need any of the special options.  Passing a NULL 
> options GHashTable should result in cfmakeraw.  Why don't you just pass all 
> the options you need instead of using tcgetattr?
> 

Should i just move the tcgetattr() to happen before cfmakeraw()?

> The other parts of the patch look good to me.

Ok. Why don't i just add the string options ("read", others...) in gattty.c?

Thanks,
Ryan
> 
> Regards,
> -Denis
> _______________________________________________
> ofono mailing list
> ofono(a)ofono.org
> http://lists.ofono.org/listinfo/ofono

Re: [PATCH] When setting the serial port, the previous settings need to be preserved.

[Denis Kenzior]

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

Hi Ryan,

> > You're blowing away the result of cfmakeraw.  Most of the devices are not
> > true serial devices and so do not need any of the special options. 
> > Passing a NULL options GHashTable should result in cfmakeraw.  Why don't
> > you just pass all the options you need instead of using tcgetattr?
>
> Should i just move the tcgetattr() to happen before cfmakeraw()?

Only one (executed last) will have effect, and I want it to be the cfmakeraw.  
So simply get rid of tcgetattr.

>
> > The other parts of the patch look good to me.
>
> Ok. Why don't i just add the string options ("read", others...) in
> gattty.c?

This is what I would prefer.

Regards,
-Denis

Re: [PATCH] When setting the serial port, the previous settings need to be preserved.

[Ryan Raasch]

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

Hello,

I have attached a small patch which allows gattty.c to enable/disable 
the read functionality on the serial port.

Cheers,
Ryan

Denis Kenzior wrote:
> Hi Ryan,
> 
>>> You're blowing away the result of cfmakeraw.  Most of the devices are not
>>> true serial devices and so do not need any of the special options. 
>>> Passing a NULL options GHashTable should result in cfmakeraw.  Why don't
>>> you just pass all the options you need instead of using tcgetattr?
>> Should i just move the tcgetattr() to happen before cfmakeraw()?
> 
> Only one (executed last) will have effect, and I want it to be the cfmakeraw.  
> So simply get rid of tcgetattr.
> 
>>> The other parts of the patch look good to me.
>> Ok. Why don't i just add the string options ("read", others...) in
>> gattty.c?
> 
> This is what I would prefer.
> 
> Regards,
> -Denis
> _______________________________________________
> ofono mailing list
> ofono(a)ofono.org
> http://lists.ofono.org/listinfo/ofono

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: serial_port_enable_CREAD.patch --]
[-- Type: text/x-patch, Size: 861 bytes --]

diff --git a/gatchat/gattty.c b/gatchat/gattty.c
index 63d26d4..f18eca4 100644
--- a/gatchat/gattty.c
+++ b/gatchat/gattty.c
@@ -97,6 +97,18 @@ static gboolean set_baud(const char *baud, struct termios *ti)
 	return TRUE;
 }
 
+static gboolean set_read(const char *bits, struct termios *ti)
+{
+	if (g_str_equal(bits, "off"))
+		ti->c_cflag &= ~(CREAD);
+	else if (g_str_equal(bits, "on"))
+		ti->c_cflag |= CREAD;
+	else
+		return FALSE;
+
+	return TRUE;
+}
+
 static gboolean set_stop_bits(const char *bits, struct termios *ti)
 {
 	if (g_str_equal(bits, "1"))
@@ -210,6 +222,8 @@ static int open_device(const char *tty, GHashTable *options)
 				ok = set_rtscts(value, &ti);
 			else if (g_str_equal(key, "local"))
 				ok = set_local(value, &ti);
+			else if (g_str_equal(key, "read"))
+				ok = set_read(value, &ti);
 
 			if (ok == FALSE)
 				return -1;

Re: [PATCH] When setting the serial port, the previous settings need to be preserved.

[Denis Kenzior]

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

Hi Ryan,

> I have attached a small patch which allows gattty.c to enable/disable
> the read functionality on the serial port.

This patch has been applied.  Thanks!

Regards,
-Denis

[PATCH] When setting the serial port, the previous settings need to be preserved.

[Ryan Raasch]

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

When setting the serial terminal before starting the daemon, this will
prevent the overwriting of settings.

Ryan

---
 gatchat/gattty.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/gatchat/gattty.c b/gatchat/gattty.c
index 63d26d4..ed0fadf 100644
--- a/gatchat/gattty.c
+++ b/gatchat/gattty.c
@@ -220,6 +220,7 @@ static int open_device(const char *tty, GHashTable *options)
 	if (fd < 0)
 		return -1;

+	tcgetattr(fd,&ti);
 	tcflush(fd, TCIOFLUSH);
 	tcsetattr(fd, TCSANOW, &ti);

-- 
1.6.4.GIT

Re: [PATCH] When setting the serial port, the previous settings need to be preserved.

[Ryan Raasch]

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

Oops. Sorry for duplicate email. I didn't get it in my inbox.

Ryan

Ryan Raasch wrote:
> When setting the serial terminal before starting the daemon, this will
> prevent the overwriting of settings.
> 
> Ryan
> 
> ---
>  gatchat/gattty.c |    1 +
>  1 files changed, 1 insertions(+), 0 deletions(-)
> 
> diff --git a/gatchat/gattty.c b/gatchat/gattty.c
> index 63d26d4..ed0fadf 100644
> --- a/gatchat/gattty.c
> +++ b/gatchat/gattty.c
> @@ -220,6 +220,7 @@ static int open_device(const char *tty, GHashTable *options)
>  	if (fd < 0)
>  		return -1;
> 
> +	tcgetattr(fd,&ti);
>  	tcflush(fd, TCIOFLUSH);
>  	tcsetattr(fd, TCSANOW, &ti);
>