Bug in EIB4 (11.001- date) handling

[Alexander Szekely]

Hello,

I think I've found a bug in linknx regarding the handling of
EIB4 datapoints.

The KNX standard states, that the year is encoded as follows:

Value >= 90 ... interpret as 19xx
Vale < 90 ... interpret as 20xx
Only years from 1990 to 2089 are supported.

Linknx does not follow this convention, and therefore sends
a wrong date to the bus for years > 1999. Eg 2008 is encoded
as 6C (2008-1900=0x6C), but correct would be 08.

I'm astonished why nobody else got caught by this bug. For
example the Berker B.IQ RTR does not understand the date
encoding generated by linknx.

I've a patch ready against the current CVS (2008-06-26) version
of linknx, but I don't know where to upload.

[jef2000]

Hi,

You're right. I'll fix it for next release. You can post the patch or send it at jef2000[at]ouaye.net

In the first document I read about data types, at the time a wrote that code, it was not so clear. The explanation for the range was: 255 = year 2155, 0 = year 1900. In a more recent document that can be found on knx.org website (03_07_02 Datapoint Types v13 AS.pdf), it has been removed and only the interpretation you give is remaining.

Regards,

Jean-François

[Alexander Szekely]

Hi Jean-François,

Now I understand why I couldn't find other implementations handling the date offset as described in the document.
You can find the patch attached to this post (I hope it survives copy&paste)

If you are interested, I've almost finished the integration of SQLite as persistence store (as an additional option to the mysql and the file based store). In addition I'm working on scripting support for linknx, that allows to define "event-handlers" for actions and real functions for conditions. It needs some more work, but seems promising.

BTW: "make check" is broken for the current CVS version, that's why the patch isn't tested with CPPUNIT.

regards,
alex

------
FILE: linknx-EIS4.patch

Handle EIS4 values (11.001) according to
KNX standard Datapoint Types v13 AS.doc 2007.03.19

The EIS4 (11.001) type only supportes years from 1990 to 2089. Hence,
values 90..99 are interpreted as 19xx, and values 00..89 are interpreted
as 20xx.

Tested against Berker B.IQ RTR, CPPUNIT tests not checked.

Diff against CVS checkout from 2008-06-26

Alexander Szekely <eib@@AT@@astech.at>

diff -Naur linknx/src/objectcontroller.cpp linknx-EIS4/src/objectcontroller.cpp
--- linknx/src/objectcontroller.cpp     2008-03-10 00:42:59.000000000 +0100
+++ linknx-EIS4/src/objectcontroller.cpp        2008-06-26 17:03:05.000000000 +0200
@@ -437,12 +437,16 @@
         return;
     std::istringstream val(value);
     char s1, s2;
-    val >> year_m >> s1 >> month_m >> s2 >> day_m;
-    year_m -= 1900;
+    int year;
+    val >> year >> s1 >> month_m >> s2 >> day_m;
+    // EIS4 (11.001) supportes years from 1990 to 2089
+    // values 90..99 are interpreted as 19xx, values 00..89 are interpreted as 20xx
+    year_m = year % 100;
+
     if ( val.fail() ||
          val.peek() != std::char_traits<char>::eof() || // workaround for wrong val.eof() flag in uClibc++
          s1 != '-' || s2 != '-' ||
-         year_m < 0 || year_m > 255 || month_m < 1 || month_m > 12 || day_m < 1 || day_m > 31)
+         year < 1990 || year > 2089 || month_m < 1 || month_m > 12 || day_m < 1 || day_m > 31)
     {
         std::stringstream msg;
         msg << "DateObjectValue: Bad value: '" << value << "'" << std::endl;
@@ -450,12 +454,13 @@
     }
}

+
std::string DateObjectValue::toString()
{
     if (day_m == -1)
         return "now";
     std::ostringstream out;
-    out << year_m+1900 << "-" << month_m << "-" << day_m;
+    out << (year_m > 89 ? year_m+1900 : year_m+2000) << "-" << month_m << "-" << day_m;
     return out.str();
}

@@ -467,7 +472,7 @@
         struct tm * timeinfo = localtime(&t);
         *day = timeinfo->tm_mday;
         *month = timeinfo->tm_mon+1;
-        *year = timeinfo->tm_year;
+        *year = timeinfo->tm_year % 100;
     }
     else
     {
@@ -1069,11 +1074,10 @@
     day = buf[2];
     month = buf[3];
     year = buf[4];
-    if (year < 90)
-        year += 100;
+
     if (!init_m || day != day_m || month != month_m || year != year_m)
     {
-        std::cout << "New value " << year+1900 << "-" << month << "-" << day << " for date object " << getID() << std::endl;
+        std::cout << "New value " << (year > 89 ? year+1900 : year+2000) << "-" << month << "-" << day << " for date object " << getID() << std::endl;
         day_m = day;
         month_m = month;
         year_m = year;
@@ -1085,7 +1089,7 @@
void DateObject::setDate(time_t time)
{
     struct tm * timeinfo = localtime(&time);
-    setDate(timeinfo->tm_mday, timeinfo->tm_mon+1, timeinfo->tm_year);
+    setDate(timeinfo->tm_mday, timeinfo->tm_mon+1, timeinfo->tm_year%100);
}

void DateObject::doSend(bool isWrite)
@@ -1096,8 +1100,7 @@

void DateObject::setDate(int day, int month, int year)
{
-    if (year >= 1900)
-        year -= 1900;
+
     if (!init_m ||
             day_m != day ||
             month_m != month ||
@@ -1105,7 +1108,7 @@
             (flags_m & Force))
     {
         std::cout << "DateObject: setDate "
-        << year + 1900 << "-"
+        << (year > 89 ? year+1900 : year+2000) << "-"
         << month << "-"
         << day << std::endl;
         day_m = day;
@@ -1123,12 +1126,10 @@
{
     *day = day_m;
     *month = month_m;
-    if (year_m < 1900)
-        *year = 1900 + year_m;
-    else
-        *year = 1900;
+    *year = (year_m > 89 ? year_m+1900 : year_m+2000);
}

+
ValueObject::ValueObject() : value_m(0)
{}

diff -Naur linknx/test/ObjectTest.cpp linknx-EIS4/test/ObjectTest.cpp
--- linknx/test/ObjectTest.cpp  2008-03-09 23:28:28.000000000 +0100
+++ linknx-EIS4/test/ObjectTest.cpp     2008-06-26 16:10:19.000000000 +0200
@@ -645,10 +645,10 @@
         ObjectValue* val;
         DateObject t, t2;
         int day, month, year;
-        t.setValue("1900-01-01");
-        CPPUNIT_ASSERT(t.getValue() == "1900-1-1");
+        t.setValue("1990-01-01");
+        CPPUNIT_ASSERT(t.getValue() == "1990-1-1");
         t2.setValue("now");
-        CPPUNIT_ASSERT(t2.getValue() != "1900-1-1");
+        CPPUNIT_ASSERT(t2.getValue() != "1990-1-1");

         t.setValue("2007-10-31");
         CPPUNIT_ASSERT(t.getValue() == "2007-10-31");
@@ -814,11 +814,11 @@

         Object *res = Object::create(&pConfig);
         CPPUNIT_ASSERT(res->getValue() == "2007-5-30");
-        res->setValue("1978-06-16");
+        res->setValue("1998-06-16");
         delete res;

         Object *res2 = Object::create(&pConfig);
-        CPPUNIT_ASSERT(res2->getValue() == "1978-6-16");
+        CPPUNIT_ASSERT(res2->getValue() == "1998-6-16");
         res2->setValue("now");
         delete res2;

-----

[Ben]

Hi,

I've got a B.IQ RTR and I'm very interesting about your patch.

How can I get and install it ?

Thanks for your great job !

Ben

[jef2000]

Hi,

For the internal storage of year in DateObject, I prefer to keep the previous format because it's much easier to understand and use. It's closer the the "struct tm" data structure used by libc time/date functions and less error prone (for example, with the solution you proposed, the DateObject::compare is broken).

I would prefer to adapt the DateObject::doSend method to change only the way it is encoded for bus transmission.

For the sqlite persistency, I would be happy to add it as long as it can be enabled or disabled by an option in configure script.

I'm also working on scripting support. Can you tell me more about your solution?

Regards,

Jean-François

[Alexander Szekely]

I also thought about a different storage format. However, I think it would be probably even better to store the year without an offset (be it 1900 or 1990) or simply use the struct tm. Having three different formats (one that is transfered over the bus, one for internal storage and one for C) makes it not easier. But that's only my opinion, feel free to use whatever format you want ;-)

I've added a configure option, and will post a patch after I have done some clean-up.

My scripting support is based on on lua and tolua++ and supports scripts inside the XML configuration file. I think lua is a small, fast and powerful option for scripting, albeit having a somewhat unfamiliar syntax. At the moment only some parts are working (It's my first try to embed lua into a C++ environment) and needs some work to be really useful. I hope to find some time in the next days...

regards,
alex

[jef2000]

Hi,

For the moment, I'll keep the years with an offset of 1900, simply because it works and I have other things to spend my time on. I'll just fix the DateObject::doSend method.

I was also looking at lua for the scripting. I just discovered tolua++ and it seems interesting. Can you confirm that it's only a tool to generate some glue code between C and lua and that it's not increasing significantly the executable size. I'm very careful to keep it small and clean.

Do you already have a sample xml config file showing what could be possible with lua scripting?

Do you want to be added as developer on the sourceforge project?

Regards,

Jean-François

[jef2000]

Hi,

If you compile from sources, just do the following replacement in file linknx/src/objectcontroller.cpp and re-compile.

Replace:

void DateObject::doSend(bool isWrite)
{
    uint8_t buf[5] = { 0, (isWrite ? 0x80 : 0x40), day_m, month_m, year_m };
    Services::instance()->getKnxConnection()->write(getGad(), buf, 5);
}

by:

void DateObject::doSend(bool isWrite)
{
    uint8_t buf[5] = { 0,
                       (isWrite ? 0x80 : 0x40),
                       day_m, month_m,
                       (year_m >= 100 && year_m < 190) ? year_m-100 : year_m };
    Services::instance()->getKnxConnection()->write(getGad(), buf, 5);
}

Regards,

Jean-François

[Ben]

Jean-François,

It works !

Thanks a lot.

Ben