This is the mail archive of the
ecos-patches@sources.redhat.com
mailing list for the eCos project.
Re: eCos termios patch
- From: Jonathan Larmour <jifl at eCosCentric dot com>
- To: Rick Richardson <rickr at mn dot rr dot com>
- Cc: eCos Patches List <ecos-patches at sources dot redhat dot com>
- Date: Thu, 23 Jan 2003 05:50:15 +0000
- Subject: Re: eCos termios patch
- References: <20030122105953.A29857@mn.rr.com>
Rick Richardson wrote:
Jonathon:
I submitted this patch for termios to the eCos list back in December,
and was wondering if it will be accepted. There were no comments on it
either way.
How very odd. I see it in the list archives, but I have no record of it
and it's not like me to lose a patch. Really not like me. Wibble.
Anyway, as for the patch. In order of issue....
First of all ONLCR. Yes I see you're very right that that's broken. I
think your fix could be improved though. Could you try the attached patch
for this problem instead please? (Unified diff was unclear, so I made it a
context patch). I'm afraid I haven't got the hardware setup right now to
do so myself.
The reason is that the return value wasn't right anyway, and it made sense
to conflate the two cyg_io_writes into one. I decided the variable names
were unclear too, and could get rid of actually_written. Oh and a more
recent standard to the one I had been using before clarifies that OPOST
should be set for ONLCR to take effect.
Next: using termios_write for ECHO. Entirely correct.
Next: VMIN processing. Something's not quite right here. Here's the patch
excerpt again:
1 if (t->c_cc[ VMIN ] ) {
2 if ( dev_buf_conf.rx_count >= t->c_cc[ VMIN ] )
3 *len = min(*len, dev_buf_conf.rx_count);
4 else if (*len >= t->c_cc[ VMIN ] )
5 *len = t->c_cc[ VMIN ];
6 else {
7 // This case is not handled correctly yet; needs mods to
8 // the loop below to do it right. We must wait for MIN
9 // chars, but return only *len of them. As it is right
10 // now, we will wait for only *len chars to arrive.
11 }
12 } else {
13 *len = min(*len, dev_buf_conf.rx_count);
14 }
The problem is on line 4/5 that we should return more than VMIN bytes if
VMIN bytes are already available from the device, up to a maximum of *len
obviously. So capping *len at VMIN bytes isn't the right thing to do. So
lines 4/5 shouldn't be there.
But in any case I've looked closer at handling VMIN and as I'm sure you've
seen, there's this bit:
} else { // non-canonical mode
/* This is completely wrong:
if ( size+1 >= t->c_cc[ VMIN ] )
returnnow = true;*/
} // else
I commented out these lines because someone on ecos-discuss told me it was
all completely wrong. At the time I was too busy to argue and just did it,
but now I'm not so sure. In fact I'm certain I've found the problem with
it - it simply didn't check for MIN==0.
So based on all the above I've included my suggested patch for all this as
an alternative to yours. You'll need to patch a fresh version of
termiostty.c from CVS.
Let me know if it works for you. Sorry I wasn't able to test it myself.
Jifl
--
eCosCentric http://www.eCosCentric.com/ <info@eCosCentric.com>
--[ "You can complain because roses have thorns, or you ]--
--[ can rejoice because thorns have roses." -Lincoln ]-- Opinions==mine
Index: termiostty.c
===================================================================
RCS file: /cvs/ecos/ecos/packages/io/serial/current/src/common/termiostty.c,v
retrieving revision 1.3
diff -u -5 -p -c -r1.3 termiostty.c
*** termiostty.c 23 May 2002 23:06:28 -0000 1.3
--- termiostty.c 23 Jan 2003 05:48:14 -0000
*************** set_attr( struct termios *t, struct term
*** 521,534 ****
#endif
ICRNL|IGNBRK|IGNCR|IGNPAR|
INLCR|INPCK|ISTRIP|PARMRK );
ptermios->c_oflag &= ~(OPOST|ONLCR);
! ptermios->c_oflag |= t->c_oflag & ONLCR;
! #ifdef NOTYET
! ptermios->c_oflag |= t->c_oflag & OPOST;
! #endif
ptermios->c_cflag &= ~(CLOCAL|CREAD|HUPCL);
ptermios->c_cflag |= t->c_cflag & (CLOCAL|CREAD|HUPCL);
ptermios->c_lflag &= ~(ECHO|ECHOE|ECHOK|ECHONL|ICANON|
--- 521,531 ----
#endif
ICRNL|IGNBRK|IGNCR|IGNPAR|
INLCR|INPCK|ISTRIP|PARMRK );
ptermios->c_oflag &= ~(OPOST|ONLCR);
! ptermios->c_oflag |= t->c_oflag & (OPOST|ONLCR);
ptermios->c_cflag &= ~(CLOCAL|CREAD|HUPCL);
ptermios->c_cflag |= t->c_cflag & (CLOCAL|CREAD|HUPCL);
ptermios->c_lflag &= ~(ECHO|ECHOE|ECHOK|ECHONL|ICANON|
*************** static Cyg_ErrNo
*** 597,632 ****
termios_write(cyg_io_handle_t handle, const void *_buf, cyg_uint32 *len)
{
cyg_devtab_entry_t *t = (cyg_devtab_entry_t *)handle;
struct termios_private_info *priv = (struct termios_private_info *)t->priv;
cyg_io_handle_t chan = (cyg_io_handle_t)priv->dev_handle;
! cyg_int32 size, bytes_successful, actually_written;
cyg_uint8 xbuf[WRITE_BUFSIZE];
cyg_uint8 *buf = (cyg_uint8 *)_buf;
Cyg_ErrNo res;
! size = 0;
! bytes_successful = 0;
! actually_written = 0;
! while (bytes_successful++ < *len) {
! xbuf[size] = *buf++;
! if ( (xbuf[size] == '\n') && (priv->termios.c_oflag & ONLCR) ) {
! xbuf[size] = '\r';
}
! size++;
! if ((size == (WRITE_BUFSIZE-1)) ||
! (bytes_successful == *len)) {
res = cyg_io_write(chan, xbuf, &size);
if (res != ENOERR) {
! *len = actually_written;
return res;
}
! actually_written += size;
! size = 0;
}
}
! *len = actually_written;
return ENOERR;
}
//==========================================================================
--- 594,628 ----
termios_write(cyg_io_handle_t handle, const void *_buf, cyg_uint32 *len)
{
cyg_devtab_entry_t *t = (cyg_devtab_entry_t *)handle;
struct termios_private_info *priv = (struct termios_private_info *)t->priv;
cyg_io_handle_t chan = (cyg_io_handle_t)priv->dev_handle;
! cyg_int32 xbufsize, input_bytes_read;
cyg_uint8 xbuf[WRITE_BUFSIZE];
cyg_uint8 *buf = (cyg_uint8 *)_buf;
Cyg_ErrNo res;
! xbufsize = input_bytes_read = 0;
! while (input_bytes_read++ < *len) {
! if ( (*buf == '\n') && (priv->termios.c_oflag & (OPOST|ONLCR)) ) {
! xbuf[xbufsize++] = '\r';
}
! xbuf[xbufsize++] = *buf;
! if ((xbufsize >= (WRITE_BUFSIZE-1)) || (input_bytes_read == *len) ||
! (*buf == '\n'))
! {
! cyg_int32 size = xbufsize;
res = cyg_io_write(chan, xbuf, &size);
if (res != ENOERR) {
! *len = input_bytes_read - (xbufsize - size);
return res;
}
! xbufsize = 0;
}
+ buf++;
}
! // Everything sent, so *len is correct.
return ENOERR;
}
//==========================================================================
*************** termios_read(cyg_io_handle_t handle, voi
*** 756,768 ****
if ( t->c_iflag & INLCR )
c = '\r';
returnnow = true; // FIXME: true even for INLCR?
} // else if
} else { // non-canonical mode
! /* This is completely wrong:
! if ( size+1 >= t->c_cc[ VMIN ] )
! returnnow = true;*/
} // else
#ifdef CYGSEM_IO_SERIAL_TERMIOS_USE_SIGNALS
if ( (t->c_lflag & ISIG) && (t->c_cc[ VINTR ] == c) ) {
discardc = true;
--- 752,763 ----
if ( t->c_iflag & INLCR )
c = '\r';
returnnow = true; // FIXME: true even for INLCR?
} // else if
} else { // non-canonical mode
! if ( t->c_cc[ VMIN ] && (size+1 >= t->c_cc[ VMIN ]) )
! returnnow = true;
} // else
#ifdef CYGSEM_IO_SERIAL_TERMIOS_USE_SIGNALS
if ( (t->c_lflag & ISIG) && (t->c_cc[ VINTR ] == c) ) {
discardc = true;
*************** termios_read(cyg_io_handle_t handle, voi
*** 791,801 ****
if (!discardc) {
buf[size++] = c;
if ( t->c_lflag & ECHO ) {
clen = 1;
// FIXME: what about error or non-blocking?
! cyg_io_write( chan, &c, &clen );
}
}
cyg_drv_mutex_unlock( &priv->lock );
} // while
--- 786,796 ----
if (!discardc) {
buf[size++] = c;
if ( t->c_lflag & ECHO ) {
clen = 1;
// FIXME: what about error or non-blocking?
! termios_write( handle, &c, &clen );
}
}
cyg_drv_mutex_unlock( &priv->lock );
} // while