Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Incorrect C header declaration in LIFX Protocol V2 Message headers docs #24

Open
xoblite opened this issue Nov 4, 2020 · 3 comments
Open

Comments

@xoblite
Copy link

xoblite commented Nov 4, 2020

Hi,

please be aware that at least with some compilers (e.g. MSVC Community 2019 and its bundled toolchain), the C header declaration included as part of the LIFX Protocol V2 Message headers documentation is incorrect in that it doesn't lead to the intended/required packing of the header, specifically the following part of the frame typedef...

uint16_t   protocol : 12;
uint8_t   addressable : 1;
uint8_t   tagged : 1;
uint8_t   origin : 2;

...which instead should be all of the same type uint16_t in order not to confuse the compiler...

uint16_t   protocol : 12;
uint16_t   addressable : 1;
uint16_t   tagged : 1;
uint16_t   origin : 2;

Thanks for making the LAN protocol option available; keep up the good work!

BR//KHH (@xoblite)

@delfick
Copy link

delfick commented Dec 31, 2020

I'm unfamiliar with MSVC, what does it complain about? Surely it doesn't matter if it's a uint8 or unit16 for these given they specify only a small portion of bits anyway?

@pkruger
Copy link

pkruger commented Jan 3, 2021

Haven't used MSVC in a while. I think the issue comes in with how the compiler maps the bits, i.e. it is expecting to map all the bits into a uint16_t, i.e. the same uint16_t, and having uint8_t in there as well seems to imply they additional bits should go into another variable which doesn't make sense since there is none. We can definitely look info this.

@delfick
Copy link

delfick commented Jan 4, 2021

@xoblite if you have the following, does MSVC complain?

typedef struct {
  /* frame */
  uint16_t size;
  uint16_t protocol : 12;
  bool  addressable : 1;
  bool  tagged : 1;
  uint16_t  origin : 2;
  uint32_t source;
  /* frame address */
  char  target[8];
  char  reserved_1[6];
  bool  res_required : 1;
  bool  ack_required : 1;
  uint8_t  reserved_2 : 6;
  uint8_t  sequence;
  /* protocol header */
  uint64_t reserved_3;
  uint16_t type;
  uint16_t reserved_4;
  /* variable length payload follows */
} Header;

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants