Wiegand 26-Bit Calculator - Transforming a decimal number into an 26-Bit binary number I made a 26-bit binary Wiegand calculator for an ESP32 following this format:


This is "Arduino C" :

unsigned int * getWiegand(unsigned int dec) {
  unsigned int* wiegandNum = new unsigned int[26];

  Serial.println();
  // transform dec number into binary number and store it in binaryNum[]
  int n = 1;
  for(int i = 23; i&gt;=0; i--) {
    int k = dec &gt;&gt; i;
    if (k &amp; 1)
      wiegandNum[n++] = 1;
    else 
      wiegandNum[n++] = 0;
  }
  // check for parity of the first 12 bits
  bool even = true;
  for(int i=1;i&lt;13;i++) {
    if(wiegandNum[i] == 1) {
      even = !even;
    }
  }
  // add 0 or 1 as first bit (leading parity bit - even) based on the number of ones in the first 12 bits
  if(even) {
    wiegandNum[0] = 0;
  } else {
    wiegandNum[0] = 1;
  }

  // check for parity of the last 12 bits
  bool odd = false;
  for(int i=13; i&lt;25;i++) {
    if(wiegandNum[i] == 1) {
      odd = !odd;
    }
  }
  // add 0 or 1 as last bit (trailing parity bit - odd) based on the number of ones in the last 12 bits
  if(odd) {
    wiegandNum[25] = 0;
  } else {
    wiegandNum[25] = 1;
  }

  return wiegandNum;
}

void setup() {
  Serial.begin(115200);

  unsigned int* wiegandArray = getWiegand(12679548);
  for(int i=0;i&lt;26;i++) {
    Serial.print(wiegandArray[i]);
  }
}


12679548 will return 01100000101111001011111000 which, based on this online tool is correct.

Execution time for this function is 255 microseconds. I don't know if the time is good or not, but I believe that the code can be improved.

What do you think ?

References:


https://www.arduino.cc/reference/en/language/structure/sketch/setup/
https://www.arduino.cc/reference/en/language/functions/communication/serial/print/
Here are a number of ideas that may help you improve your program.

Separate calculation from I/O

The Serial.println(); call from within getWeigand really shouldn't be there.  It's better if the function does just one thing, and that is to calculate and return the number.

Don't leak memory

The code calls new but there isn't any matching delete which means that the program leaks memory.  It's better to make sure all memory is freed, but better still, see the next suggestion.

Avoid using new and delete

Modern C++ doesn't use new and delete as much as it used to.  In this case, much better than passing around a raw array pointer and hoping the other end knows what the size is, is to use std::array instead.  That is an improvement over a plain array because it works with all of the standard container algorithms and has its own .size() operator. See I.13 for more details on why passing around a single array pointer is not a good idea.

Prefer single bit shifting to multi-bit shifting

Many processors have hardware optimizations for multi-bit shifts, but not all do.  That means on some machines, multi-bit shifts take longer.  For that reason, and because I think it's more readable, I'd suggest changing the first for loop to this:

for (int i = 24; i &gt; 0; --i) {
    wiegandNum[i] = dec &amp; 1;
    dec &gt;&gt;= 1;
}


Simplify your code

Instead of doing things like this:

if(even) {
    wiegandNum[0] = 0;
} else {
    wiegandNum[0] = 1;
}


just use the value directly:

wiegandNum[0] = !even;


Use whitespace for readability

Lines like this one:

for(int i=1;i&lt;13;i++) {


are much easier to read with a little more whitespace:

for (int i = 1; i &lt; 13; i++) {


Create a test routine

With code like this, it's usually good to do extensive testing to make sure it does what you intend.  I am not using an Arduino, but created a version that runs on Linux (or really any computer that supports C++):

#include &lt;string&gt;
#include &lt;iostream&gt;
#include &lt;iomanip&gt;
std::string weiString(unsigned int dec)
{
    std::string str{};
    auto answer = getWiegand(dec);
    for (const auto digit: answer) {
        str.push_back(digit ? '1' : '0');
    }
    return str;
}

int main()
{
    struct {
        unsigned n;
        std::string expected;
        bool operator()() const {
            std::string calculated = weiString(n);
            bool ok{calculated == expected};
            std::cout &lt;&lt; std::hex &lt;&lt; n 
                &lt;&lt; std::dec 
                &lt;&lt; '\t' &lt;&lt; expected 
                &lt;&lt; '\t' &lt;&lt; calculated 
                &lt;&lt; '\t' &lt;&lt; (ok ? "OK" : "incorrect!") 
                &lt;&lt; '\n';
            return ok;
        }
    } tests[]{
        {12679548,  "01100000101111001011111000" },
        {0xffff,    "00000000011111111111111111" },
        {0x1ffff,   "10000000111111111111111111" },
        {0xffffff,  "01111111111111111111111111" },
        {0xdecade,  "01101111011001010110111101" },
        {0xfffffff, "01111111111111111111111111" },  // ignore high bits
    };

    for (const auto&amp; test: tests) {
        test();
    }
}


Results

Here's the alternate version that uses these suggestions.  It also uses the exclusive-or operator ^ to simplify parity calculations:

#include &lt;array&gt;

std::array&lt;unsigned, 26&gt; getWiegand(unsigned int dec)
{
    std::array&lt;unsigned, 26&gt; wiegandNum;
    for (int i = 24; i &gt; 0; --i) {
        wiegandNum[i] = dec &amp; 1;
        dec &gt;&gt;= 1;
    }
    // check for parity of the first 12 bits
    bool even = false;
    for (int i = 1; i &lt; 13; i++) {
        even ^= wiegandNum[i];
    }
    wiegandNum[0] = even;

    // check for parity of the last 12 bits
    bool odd = true;
    for (int i = 13; i &lt; 25; i++) {
        odd ^= wiegandNum[i];
    }
    wiegandNum[25] = odd;

    return wiegandNum;
}


Even shorter is to process one of the parity bits over the entire array as it's generated and then recalculate just the other one and adjust.  It might be less obvious why this works, but it does.  See if you can see why.

std::array&lt;unsigned, 26&gt; getWiegand(unsigned int dec)
{
    bool odd = true;
    std::array&lt;unsigned, 26&gt; wiegandNum;
    for (int i = 24; i &gt; 0; --i) {
        wiegandNum[i] = dec &amp; 1;
        odd ^= wiegandNum[i];
        dec &gt;&gt;= 1;
    }
    // check for parity of the first 12 bits
    bool even = false;
    for (int i = 1; i &lt; 13; i++) {
        even ^= wiegandNum[i];
    }
    wiegandNum[0] = even;
    wiegandNum[25] = odd ^ even;
    return wiegandNum;
}