[vz-dev] vzlogger: wieder ein bug im d0-parser

[Thorben Thuermer]

und num zum bugs fixen...

On Thu, 10 Jan 2013 23:01:30 +0100 Thorben Thuermer <r00t at constancy.org> wrote:
> der zaehler sendet am anfang des telegramms:
> 0000000: 2f41 4242 345c 4056 352e 3230 2039 3036  /ABB4\@V5.20 906
> 0000010: 3036 3839 330d 0a02 3332 2e37 2e30 2832  06893...32.7.0(2
> 
> d.h. die start-zeile, dann \r\n (0x0d, 0x0a), dann ein byte: 0x02,
> und erst dann die ascii-ziffern der ersten obis-ID.
> 0x02 ist laut doku des zaehlerherstellers in "<STX>" "start of text" marker.
> 
> das problem ist nun, das vzlogger davon nix weiss, und dieses byte zur
> obis-id zaehlt:

hier ist erstmal die frage (feedback bitte!!!):
senden alle d0-zaehler das so?
(sprich, koennen wir das byte an der stelle erwarten,
 oder muessen wir es nur herausparsen FALLS es da steht?)

ich wuerde folgenden patch vorschlagen (ungetestet),
um das byte optinal herauszuparsen, falls es vorhanden ist:
--- a/src/protocols/d0.c
+++ b/src/protocols/d0.c
@@ -204,7 +204,7 @@ size_t meter_read_d0(meter_t *mtr, reading_t rds[], size_t max_readings) {
                        case IDENTIFICATION:            /* IDENTIFICATION has 16 bytes */
                                if (byte == '\r' || byte == '\n') { /* detect line end */
                                        identification[byte_iterator] = '\0'; /* termination */
-                                       context = OBIS_CODE;    /* set new context: IDENTIFICATION -> OBIS_CODE */
+                                       context = STX;
                                        byte_iterator = 0;
                                }
                                else identification[byte_iterator++] = byte;

@@ -212,6 +212,10 @@ size_t meter_read_d0(meter_t *mtr, reading_t rds[], size_t max_readings) {
 
                        case START_LINE:
                                break;
+                       case STX:
+                               context = OBIS_CODE;
+                               if (byte == 0x02) break;
+                               // fall through
                        case OBIS_CODE:
                                if ((byte != '\n') && (byte != '\r')) 
                                {

> und weiter:
[...]
> d.h., wenn das erste byte auf nix matcht (was bei 0x02 vorkommt),
> dann schreibt obis_parse nix in den rueckgabepuffer und liefert ERR zurueck,
> aber wie wir oben sehen:
> src/protocols/d0.c:
> 260                         obis_parse(obis_code, &rds[number_of_tuples].identifier.obis);
> wird der rueckgabewert ignoriert!
> 
> wir haben jetzt also nicht initialisierte daten fuer den obis-identifier...!

ich sehe hier keine intelligente methode das zu behandeln, da 'rds' ein array ist,
und wir den fehlerhaften eintrag nicht einfach entfernen koennen...
(ist aber in der praxis wohl auch nicht SO relevant).
wir sollten nur dafuer sorgen, das ein unschaedlicher wert fuer den obis-code
zurueckbleint... 0.0.0 zB?

> weiter dann:
[...]
> src/obis.c:
> 177 int obis_compare(obis_id_t a, obis_id_t b) {
> 178     for (int i = 0; i < 6; i++) {
> 179         if (a.raw[i] == b.raw[i] || a.raw[i] == 0xff || b.raw[i] == 0xff ) {
> 180             continue; /* skip on wildcard or equal */
> 190     return 0; /* equal */
> 
> durch diese (etwas zweifelhafte?!) wildcard-logik matcht die fehlerhaft
> geparsete id dann immer, und wird fuer jeden channel submitted!

ich wuerde diese wildcard-logik einfach ersatzlos entfernen...
oder gibt es irgendeine berechtigung dafuer?
in jedem fall sollten wildcards nur auf EINER seite des vergleichs erlaubt sein,
nicht auf beiden.

--- a/src/obis.c
+++ b/src/obis.c
@@ -176,8 +176,8 @@ int obis_unparse(obis_id_t id, char *buffer, size_t n) {
 
 int obis_compare(obis_id_t a, obis_id_t b) {
        for (int i = 0; i < 6; i++) {
-               if (a.raw[i] == b.raw[i] || a.raw[i] == 0xff || b.raw[i] == 0xff ) {
-                       continue; /* skip on wildcard or equal */
+               if (a.raw[i] == b.raw[i]) {
+                       continue; /* skip on equal */
                }
                else if (a.raw[i] < b.raw[i]) {
                        return -1;


 - Thorben