On 05.08.2008, at 12:17, Vincent CHAVANIS wrote:
Hi all,
A bug still exists on the UCP/EMI driver.
-Keepalive was wrong because the sender thread was awaken keepalive
+ 1
When you set keepalive=5 you will in reality have keepalive=6
so you say we where off by one second?
-Then, the patch also fixes the UCP31 at login.
We don't need to send it beacause UCP60 already make an activity on
the SMSC line. (The issue is that we need to wait_for_ack in order
to send MT = waste of time). Note that UCP31 is an alert operation
which can be used for a TCP keepalive.
This is not true. UCP31 is not in fact a keepalive. its used to alert
the SMSC that we're ready to receive incoming SMS.
Login by itself COULD do this but not necessarily. Also there might be
some situation where login is not done at all.
From the point of view of <Bruno Rodrigues> who dit the previous
patch (2002-03-14) "a bug when Kannel couldn't connect to the server
on the first time, and then on a later attempt, the last_activity
was wrong and Kannel wouldn't send the 31's, or would send them at
the wrong time. [...] As a work around I made Kannel always send a
31 after a successful login to make sure the timers were reset."
This does not seems to happens anymore.
I would leave it in there. It doesn't hurt and probably helps some
specific SMSC implementations.
Any comments ?
@@ -158,7 +159,7 @@
#define
emi2_needs_keepalive
(conn) \
(emi2_can_send(conn)
&& \
(PRIVDATA(conn)->keepalive > 0)
&& \
- (time(NULL) > (PRIVDATA(conn)->last_activity_time + PRIVDATA(conn)-
>keepalive)))
+ (time(NULL) >= (PRIVDATA(conn)->last_activity_time +
PRIVDATA(conn)->keepalive)))
so this is keepalive always being +1... sounds good to me.
/*
* Send an EMI message and update the last_activity_time field.
@@ -366,7 +373,7 @@
connect_error = 1;
continue;
}
- privdata->last_activity_time = 0; /* to force keepalive
after login */
+ privdata->last_activity_time = time (NULL); /* to *NOT*
force keepalive after login */
as I said above, I would skip that one.
} else if (emimsg->ot == 31) {
+ PRIVDATA(conn)->last_activity_time = time (NULL);
/* XXX Process error codes here
if (octstr_get_char(emimsg->fields[0], 0) == 'N') {
long errorcode;
not sure why this is needed. Would sound like it would have never
worked otherwise...
@@ -1269,7 +1317,7 @@
*/
static double emi2_get_timeouttime (SMSCConn *conn, Connection
*server)
{
- double ka_timeouttime = PRIVDATA(conn)->keepalive ?
PRIVDATA(conn)->keepalive + 1 : DBL_MAX;
+ double ka_timeouttime = PRIVDATA(conn)->keepalive ?
PRIVDATA(conn)->keepalive : DBL_MAX;
shouldn't timeout be longer than keepalive? could above change not
create a race condition to the fact that the keepalive was "just about
to be sent" but timeout hit first?
double idle_timeouttime = (PRIVDATA(conn)->idle_timeout &&
server) ? PRIVDATA(conn)->idle_timeout : DBL_MAX;
double result = ka_timeouttime < idle_timeouttime ?
ka_timeouttime : idle_timeouttime;
@@ -1626,7 +1674,7 @@
privdata->listening_socket = -1;
privdata->can_write = 1;
privdata->priv_nexttrn = 0;
- privdata->last_activity_time = 0;
+ privdata->last_activity_time = time (NULL); /* to *NOT* force
keepalive after login */
as I said I would leave keepalive at login.
Andreas Fink
Fink Consulting GmbH
Global Networks Schweiz AG
BebbiCell AG
IceCell ehf
---------------------------------------------------------------
Tel: +41-61-6666330 Fax: +41-61-6666331 Mobile: +41-79-2457333
Address: Clarastrasse 3, 4058 Basel, Switzerland
E-Mail: [EMAIL PROTECTED]
www.finkconsulting.com www.global-networks.ch www.bebbicell.ch
---------------------------------------------------------------
ICQ: 8239353 MSN: [EMAIL PROTECTED] AIM: smsrelay Skype: andreasfink
Yahoo: finkconsulting SMS: +41792457333
http://a-fink.blogspot.com/ A developers view about iPhone SDK
