Serial.write() with Bitfields

Perehama · September 18, 2020, 12:43pm

I have the following structs in a master unit used to compress a lot of information into as few bytes as possible.

struct fourZoneStruct {
    unsigned char zone1 : 2;
    unsigned char zone2 : 2;
    unsigned char zone3 : 2;
    unsigned char zone4 : 2;
};

struct bellEventStruct {
	unsigned char startHour : 5;
	unsigned char startMinute : 6;
	unsigned char dowSunday : 1;
	unsigned char dowMonday : 1;
	unsigned char dowTuesday : 1;
	unsigned char dowWednesday : 1;
	unsigned char dowThursday : 1;
	unsigned char dowFriday : 1;
	unsigned char dowSaturday : 1;
	fourZoneStruct zones1to4;
} bellEvent[50];

Each "Event" has a given start time in hours and minutes within a 24 hour day and can happen on any of 7 days of a week and affect 4 "zones". Each zone can take 1 of 4 actions, with action 0 being no action.
This is used in a master unit that keeps track of the time, date and master schedule. When there is an event upcoming, the master unit needs to send a serial command to a secondary unit, which keeps time of day and can take action on the "zone" within that 24-hour day. The secondary unit stores the same information in the following struct.

struct fourZoneStruct {
    unsigned char zone1 : 2;
    unsigned char zone2 : 2;
    unsigned char zone3 : 2;
    unsigned char zone4 : 2;
};

struct bellEventStruct {
	unsigned char startHour : 5;
	unsigned char startMinute : 6;
	fourZoneStruct zones1to4;
} bellEvent[50];

I would like the protocol for this communication to be 4 bytes, the first being ASCII 'E' to signify the start of the message, the second being the hour value. The third being the minute value and the 4th containing the zone data. It is the 4th byte I am having difficulty with and would like some help.

How can I Serial.write() the byte stored inbellEvent[i].zones1to4?

When I have sent it, how can I set the value of
bellEvent[i].zones1to4 to the value returned by Serial.read() in the secondary unit so that the zone information may be communicated in 1 byte and the bit fields preserved?

pylon · September 18, 2020, 2:04pm

Just to be sure: the two "units" are identical boards and the sketches are compiled on the same IDE. Is that correct?

How can I Serial.write() the byte stored in

The correct way would be to form a unit which offers byte output.

A quick hack is:

Serial.write(*((uint8_t *)&(bellEvent[i].zones1to4)));

Just to have asked: Is it worth the effort to transfer a few bytes less?

Perehama · September 18, 2020, 2:29pm

pylon:
Just to be sure: the two "units" are identical boards and the sketches are compiled on the same IDE. Is that correct?

NO Same IDE, Arduino 1.8.12

The correct way would be to form a unit which offers byte output.

A quick hack is:
Serial.write(*((uint8_t *)&(bellEvent[i].zones1to4)));
Just to have asked: Is it worth the effort to transfer a few bytes less?

YES

I got all too far in this project before getting to this problem. I am writing a small sketch that tests this hack and the correct way in a single INO without the complexity of master/secondary.

markd833 · September 18, 2020, 2:55pm

I think you can define a union consisting of fourZoneStruct{} and an unsigned char. You can then access the same location in memory either as the individual elements of your struct or as an unsigned char.

It's been a while since I used unions, so hopefully somebody with more C experience will jump in and provide the correct syntax

Oh, I almost forgot, from memory, you might have to add a compiler directive, something like attribute((packed)) so that the compiler really does put all 4 of your 2-bit bitfields into one byte.

aarg · September 18, 2020, 3:26pm

Do not use unions for type punning. It’s thoroughly discredited. I copied this example from another thread:

uint16_t value = 0x4321;
uint8_t *as_byte_array = reinterpret_cast<uint8_t *>(&value);
uint8_t lsb = as_byte_array[0];
uint8_t msb = as_byte_array[1];

There it was also mentioned that you can safely use memcpy().

Perehama · September 18, 2020, 3:44pm

Here is a simple sketch that illustrates the problem.

unsigned char eventCount;

struct fourZoneStruct {
    unsigned char zone1 : 2;
    unsigned char zone2 : 2;
    unsigned char zone3 : 2;
    unsigned char zone4 : 2;
};

struct bellEventStruct {
  unsigned char startHour : 5;
  unsigned char startMinute : 6;
  fourZoneStruct zones1to4;
} bellEvent[50];

void setup() {
  Serial.begin(9600);
}

void loop() {
  static byte cCount = 0;
  static unsigned char buf[4] = {0, 0, 0, 0};
  static unsigned long serialInTimestamp;
  if (buf[0] > 0) {
    if (millis() - serialInTimestamp >= 200UL) {
      memset(buf, 0, sizeof(buf));
      cCount = 0;
    }
  }
  if (Serial.available()) {
    int c = Serial.read();
    if (cCount == 0) {
      if (c == 'E') {
        serialInTimestamp = millis();
        buf[cCount] = c;
        cCount++;
      }
      else {
        memset(buf, 0, sizeof(buf));
        cCount = 0;
      }
    }
    else if (cCount == 1) {
      buf[cCount] = c;
        cCount++;
    }
    else if (cCount == 2) {
      buf[cCount] = c;
        cCount++;
    }
    else if (cCount == 3) {// hours tens
      buf[cCount] = c;
        cCount++;
    }
  }
  if (cCount > 3 && buf[0] > 0) {
    bellEvent[eventCount].startHour = buf[1];
    bellEvent[eventCount].startMinute = buf[2];
    bellEvent[eventCount].zones1to4.zone1 = (buf[3] >> 6) && B00000011;
    bellEvent[eventCount].zones1to4.zone2 = (buf[3] >> 4) && B00000011;
    bellEvent[eventCount].zones1to4.zone3 = (buf[3] >> 2) && B00000011;
    bellEvent[eventCount].zones1to4.zone4 = buf[3] && B00000011;
    unsigned char x = (bellEvent[eventCount].zones1to4.zone1 << 6) + (bellEvent[eventCount].zones1to4.zone2 << 4) + (bellEvent[eventCount].zones1to4.zone3 << 2) + bellEvent[eventCount].zones1to4.zone4;
    Serial.write(x);
    if (eventCount >= 49) eventCount = 0;
    else eventCount++;
    memset(buf, 0, sizeof(buf));
    cCount = 0;
  }
}

This doesn’t work… No matter the value of the 4th bit, ‘U’ is printed…

PieterP · September 18, 2020, 4:09pm

Perehama:
Here is a simple sketch that illustrates (and offers one solution) the problem. Is this the best solution? it works!
    unsigned char x = (bellEvent[eventCount].zones1to4.zone1 << 6) + (bellEvent[eventCount].zones1to4.zone2 << 4) + (bellEvent[eventCount].zones1to4.zone3 << 2) + bellEvent[eventCount].zones1to4.zone4;

Serial.write(x);

Yes, that’s one of the better solutions. By using bit shifts, you’re sure that the result is independent of the Endianness of the different systems involved.

If Endianness is not a concern, you could use the reinterpret_cast approach, to inspect the underlying bit pattern of your struct. You do have to make sure the struct is actually one byte in size.

   unsigned char x = *reinterpret_cast<unsigned char *>(&bellEvent[eventCount].zones1to4);
   static_assert(sizeof(bellEvent[eventCount].zones1to4) == 1, "Wrong padding");
   Serial.write(x);

Note that you can only use reinterpret_cast here because you’re casting to a byte/character type. Casting to any other type is usually not allowed, and you would have to use memcpy in that case.

For example, to receive the four zones over a serial link:

   unsigned char x = Serial.read();
   static_assert(sizeof(bellEvent[eventCount].zones1to4) == 1, "Wrong padding");
   memcpy(&bellEvent[eventCount].zones1to4, &x, 1);

Using a reinterpret_cast would be wrong:

    unsigned char x = Serial.read();
   fourZoneStruct zones = *reinterpret_cast<fourZoneStruct *>(&x); // WRONG! You cannot interpret a character as a fourZoneStruct

Perehama:
Here is a simple sketch that illustrates the problem.

This doesn’t work… No matter the value of the 4th bit, ‘U’ is printed…

Then I suspect that the problem is not where you think it is. Using bit shifts works just fine to write the bit fields to the Serial port: Compiler Explorer

If ‘U’ is printed, that means that all zones are set to 1. (ASCII code for ‘U’ is 0b01010101).

Pieter

markd833 · September 18, 2020, 4:16pm

A quick test of your code and I think it'll work with your code like this:

    bellEvent[eventCount].zones1to4.zone1 = (buf[3] >> 6) & B00000011;
    bellEvent[eventCount].zones1to4.zone2 = (buf[3] >> 4) & B00000011;
    bellEvent[eventCount].zones1to4.zone3 = (buf[3] >> 2) & B00000011;
    bellEvent[eventCount].zones1to4.zone4 = buf[3] & B00000011;

Perehama · September 18, 2020, 4:22pm

PieterP:
Yes, that’s one of the better solutions. By using bit shifts, you’re sure that the result is independent of the Endianness of the different systems involved.
    unsigned char x = Serial.read();

fourZoneStruct zones = *reinterpret_cast<fourZoneStruct *>(&x); // WRONG! You cannot interpret a character as a fourZoneStruct



Then I suspect that the problem is not where you think it is. Using bit shifts works just fine to write the bit fields to the Serial port: https://godbolt.org/z/edx5jP

If 'U' is printed, that means that all zones are set to 1. (ASCII code for 'U' is `0b01010101`).

Pieter

Thank you for working on this. Sorry for editing my post on you… Endianness was exactly my concern with casting and memcopy etc. I just didn’t have the words ;-). I think I’ve almost got it, but suspect I’m doing the left shift and mask wrong…

bellEvent[eventCount].zones1to4.zone1 = (buf[3] >> 6) && B00000011;

No matter the value of buf[3] the value of each zone is always 1.

Perehama · September 18, 2020, 4:24pm

markd833:
A quick test of your code and I think it'll work with your code like this:
  bellEvent[eventCount].zones1to4.zone1 = (buf[3] >> 6) & B00000011;

bellEvent[eventCount].zones1to4.zone2 = (buf[3] >> 4) & B00000011;
bellEvent[eventCount].zones1to4.zone3 = (buf[3] >> 2) & B00000011;
bellEvent[eventCount].zones1to4.zone4 = buf[3] & B00000011;

Thanks! it's always the littlest things....

PieterP · September 18, 2020, 4:31pm

markd833:
A quick test of your code and I think it'll work with your code like this:
    bellEvent[eventCount].zones1to4.zone1 = (buf[3] >> 6) & B00000011;

bellEvent[eventCount].zones1to4.zone2 = (buf[3] >> 4) & B00000011;
bellEvent[eventCount].zones1to4.zone3 = (buf[3] >> 2) & B00000011;
bellEvent[eventCount].zones1to4.zone4 = buf[3] & B00000011;

Good catch.

You don't even need the bitmask, the bit fields are unsigned, so they behave modulo 2². The compiler adds the mask implicitly.

system · May 6, 2021, 12:45am