hhromic / libe131

libE131: a lightweight C/C++ library for the E1.31 (sACN) protocol
Apache License 2.0
76 stars 19 forks source link

e131_get_option returns the opposite of what was set using e131_set_option #4

Closed shenghaoyang closed 6 years ago

shenghaoyang commented 6 years ago

Hello,

Not sure if this is a known bug (or an user error :(), but, when compiling from a fresh clone of the master branch, e131_get_option() seems to return the opposite of what was previously set using e131_set_option().

If it isn't caused by an EBKAC, I think the problem seems to lie in this if statement condition of e131_get_option():

bool e131_get_option(const e131_packet_t *packet, const e131_option_t option) {
  if (packet == NULL || packet->frame.options & (1 << (option % 8)))
    return false;
  return true;
}

If the relevant bit is set this statement:

packet->frame.options & (1 << (option % 8))

Evaluates to true, which causes the

return false

statement to be executed.

Was going to put in a pull request with a fix, but I just wanted to make sure I was using it correctly.

Thanks for giving this a look through.

I ran this little snippet to test:

#include <e131.h>
#include <string.h>
#include <stdlib.h>

int main(int argc, char** argv) {
    e131_packet_t packet;
    e131_pkt_init(&packet, 1, 1);
    e131_set_option(&packet, E131_OPT_TERMINATED, 1);

    printf("------------Dumping packet to stdout--------------\n");
    e131_pkt_dump(stdout, &packet);
    printf("----------------End packet dump-------------------\n");
    printf("Set E131_OPT_TERMINATED, but result of the get_option function is:"
           " %d\n", e131_get_option(&packet, E131_OPT_TERMINATED));

    e131_set_option(&packet, E131_OPT_PREVIEW, 1);
    printf("------------Dumping packet to stdout--------------\n");
    e131_pkt_dump(stdout, &packet);
    printf("----------------End packet dump-------------------\n");
    printf("Set E131_OPT_PREVIEW, but result of the get_option function is:"
           " %d\n", e131_get_option(&packet, E131_OPT_PREVIEW));

    e131_set_option(&packet, E131_OPT_TERMINATED, 0);
    printf("------------Dumping packet to stdout--------------\n");
    e131_pkt_dump(stdout, &packet);
    printf("----------------End packet dump-------------------\n");
    printf("Cleared E131_OPT_TERMINATED, but result of the get_option function"
           " is: %d\n", e131_get_option(&packet, E131_OPT_TERMINATED));

    e131_set_option(&packet, E131_OPT_PREVIEW, 0);
    printf("------------Dumping packet to stdout--------------\n");
    e131_pkt_dump(stdout, &packet);
    printf("----------------End packet dump-------------------\n");
    printf("Cleared E131_OPT_PREVIEW, but result of the get_option function"
           " is: %d\n", e131_get_option(&packet, E131_OPT_PREVIEW));
}

And it gave me this result:

------------Dumping packet to stdout--------------
[Root Layer]
  Preamble Size .......... 16
  Post-amble Size ........ 0
  ACN Packet Identifier .. ASC-E1.17
  Flags & Length ......... 28783
  Layer Vector ........... 4
  Component Identifier ... 00000000000000000000000000000000
[Framing Layer]
  Flags & Length ......... 28761
  Layer Vector ........... 2
  Source Name ............ 
  Packet Priority ........ 100
  Reserved ............... 0
  Sequence Number ........ 0
  Options Flags .......... 64
  DMX Universe Number .... 1
[Device Management Protocol (DMP) Layer]
  Flags & Length ......... 28684
  Layer Vector ........... 2
  Address & Data Type .... 161
  First Address .......... 0
  Address Increment ...... 1
  Property Value Count ... 2
[DMP Property Values]
  00 00
----------------End packet dump-------------------
Set E131_OPT_TERMINATED, but result of the get_option function is: 0
------------Dumping packet to stdout--------------
[Root Layer]
  Preamble Size .......... 16
  Post-amble Size ........ 0
  ACN Packet Identifier .. ASC-E1.17
  Flags & Length ......... 28783
  Layer Vector ........... 4
  Component Identifier ... 00000000000000000000000000000000
[Framing Layer]
  Flags & Length ......... 28761
  Layer Vector ........... 2
  Source Name ............ 
  Packet Priority ........ 100
  Reserved ............... 0
  Sequence Number ........ 0
  Options Flags .......... 192
  DMX Universe Number .... 1
[Device Management Protocol (DMP) Layer]
  Flags & Length ......... 28684
  Layer Vector ........... 2
  Address & Data Type .... 161
  First Address .......... 0
  Address Increment ...... 1
  Property Value Count ... 2
[DMP Property Values]
  00 00
----------------End packet dump-------------------
Set E131_OPT_PREVIEW, but result of the get_option function is: 0
------------Dumping packet to stdout--------------
[Root Layer]
  Preamble Size .......... 16
  Post-amble Size ........ 0
  ACN Packet Identifier .. ASC-E1.17
  Flags & Length ......... 28783
  Layer Vector ........... 4
  Component Identifier ... 00000000000000000000000000000000
[Framing Layer]
  Flags & Length ......... 28761
  Layer Vector ........... 2
  Source Name ............ 
  Packet Priority ........ 100
  Reserved ............... 0
  Sequence Number ........ 0
  Options Flags .......... 128
  DMX Universe Number .... 1
[Device Management Protocol (DMP) Layer]
  Flags & Length ......... 28684
  Layer Vector ........... 2
  Address & Data Type .... 161
  First Address .......... 0
  Address Increment ...... 1
  Property Value Count ... 2
[DMP Property Values]
  00 00
----------------End packet dump-------------------
Cleared E131_OPT_TERMINATED, but result of the get_option function is: 1
------------Dumping packet to stdout--------------
[Root Layer]
  Preamble Size .......... 16
  Post-amble Size ........ 0
  ACN Packet Identifier .. ASC-E1.17
  Flags & Length ......... 28783
  Layer Vector ........... 4
  Component Identifier ... 00000000000000000000000000000000
[Framing Layer]
  Flags & Length ......... 28761
  Layer Vector ........... 2
  Source Name ............ 
  Packet Priority ........ 100
  Reserved ............... 0
  Sequence Number ........ 0
  Options Flags .......... 0
  DMX Universe Number .... 1
[Device Management Protocol (DMP) Layer]
  Flags & Length ......... 28684
  Layer Vector ........... 2
  Address & Data Type .... 161
  First Address .......... 0
  Address Increment ...... 1
  Property Value Count ... 2
[DMP Property Values]
  00 00
----------------End packet dump-------------------
Cleared E131_OPT_PREVIEW, but result of the get_option function is: 1
hhromic commented 6 years ago

Hi @shenghaoyang, well spotted! I just confirmed that indeed this is a bug, much appreciated. I got a bit messed with the logic in e131_get_option() and I fixed it now. Thanks again!

shenghaoyang commented 6 years ago

Awesome, thanks for the quick fix!

hhromic commented 6 years ago

Made a new release just now. Thanks again for your help.