bitSet only sets bits from 0 to 31 (previously to 15)

Hi

I just realised that bitSet() only works for bits 0 to 31. When wanting to set bits higher than 31, I seem to have to use bitWrite(); Is this an error or a wanted behaviour?

Regards, Dani

uint64_t sternbilder = 0;
bitSet(sternbilder, 8); // for bits from 0 to 31
bitWrite(sternbilder, 32, 1); // for bits 32 and above

From Arduino.h

#define bitSet(value, bit) ((value) |= (1UL << (bit)))
#define bitClear(value, bit) ((value) &= ~(1UL << (bit)))
#define bitWrite(value, bit, bitvalue) (bitvalue ? bitSet(value, bit) : bitClear(value, bit))

so bitWrite() is just a call to bitSet()

because it use 1UL which is essentially 32bit unsigned it will not work for uint64_t

(thinking about fix)

Are you sure about your observations?
The code for bitSet is a macro which looks long compatible to me.
Also bitWrite is a macro using bitSet.
So if bitWrite works for longs; bitSet should as well

#define bitSet(value, bit) ((value) |= (1UL << (bit)))
#define bitWrite(value, bit, bitvalue) (bitvalue ? bitSet(value, bit) : bitClear(value, bit))

Best regards
Jantje

A solution for 64bit values is to define the following macros, as they use explicit an unsigned long long as datatype.
(you include these in the Arduino.h )

#define bitSet64(value, bit) ((value) |= (1ULL << (bit)))
#define bitClear64(value, bit) ((value) &= ~(1ULL << (bit)))
#define bitWrite64(value, bit, bitvalue) (bitvalue ? bitSet(value, bit) : bitClear(value, bit))

A more elaborate solution might be

//
//    FILE: .ino
//  AUTHOR: Rob Tillaart
// VERSION: 0.1.00
// PURPOSE: 
//    DATE: 
//     URL:
//
// Released to the public domain
//

#define bitSet64(value, bit) ((value) |= (bit<32?1UL:1ULL) <<(bit))
#define bitClear64(value, bit) ((value) &= ~(bit<32?1UL:1ULL) <<(bit))
#define bitWrite64(value, bit, bitvalue) (bitvalue ? bitSet64(value, bit) : bitClear64(value, bit))


void setup() 
{
  Serial.begin(115200);
  Serial.println("Start ");

  uint64_t sternbilder = 0;

  for (int i=1; i<65; i*=2)
  {
    bitSet64(sternbilder, i-1); 
  }
  Serial.println(sternbilder, BIN);
}

void loop() {}

Note: I have patched Print.h for uint64 - http://forum.arduino.cc/index.php?topic=143584.msg1079973#msg1079973 -

Rob The existing macros should work for unsigned long, right? That is from bit 16 to 31 Your proposal is needed for bits 32 to 63 Or did I miss something? Best regards Jantje

Rob, you are so fast I'm hardly able to type in your suggestions before you have an even better one :) I patched the Print.h und Print.cpp and included your #definitions for 64bit magic an am very happy that now everything works as expected. Thank you very much - you should add this code to future arduino-IDE-releases!

Jantje, I think you are right - I think I have gotten the 15 wrong - it seems to be 31 that is the limit with the normal bitSet()-function. Please excuse my misswriting. Dani.

Dani Thanks; I started thinking I was loosing it :astonished: Best regards Jantje

Jantje, you were perfectly present!

It is better to test the sizeof(value) than to test bit, as bit often bit can be a variable so its value cannot be checked compile time.

#define bitSet64(value, bit) ((value) |= (sizeof(value)<5?1UL:1ULL) <<(bit))
#define bitClear64(value, bit) ((value) &= ~(sizeof(value)<5?1UL:1ULL) <<(bit))
#define bitWrite64(value, bit, bitvalue) (bitvalue ? bitSet64(value, bit) : bitClear64(value, bit))

posted as issue - https://github.com/arduino/Arduino/issues/1670 -

C++ allows us to do things beyond macros. How about a template class for any size bitset? With operator syntax and that is adapted for an 8-bit machine as ATmega328.

template<uint16_t N>
class bitset_t {
private:
  uint8_t m_set[(N + 4)/CHARBITS];
public:
  bitset_t() 
  {
    empty();
  }
  uint16_t members() 
  {
    return (N);
  }
  void empty()
  {
    memset(m_set, 0, sizeof(m_set));
  }
  bool operator[](uint16_t ix) 
  {
    if (ix < N) 
      return ((m_set[ix / CHARBITS] & _BV(ix & (CHARBITS - 1))) != 0);
    return (false);
  }
  void operator+=(uint16_t ix)
  {
    if (ix < N) m_set[ix / CHARBITS] |= _BV(ix & (CHARBITS - 1));
  }
  void operator-=(uint16_t ix)
  {
    if (ix < N) m_set[ix / CHARBITS] &= ~_BV(ix & (CHARBITS - 1));
  }
  void dump()
  {
    for (uint16_t i = 0; i < N; i++) 
      printf("%d", (m_set[i / CHARBITS] & _BV(i & (CHARBITS - 1))) != 0);
    printf("\n");
  }
};

A small test program (as tested on Linux).

enum {
  GREEN,
  BLUE,
  YELLOW,
  RED
};
  
int main()
{
  bitset_t<48> b;
  TRACE(b[GREEN]);
  b += GREEN;
  TRACE(b[GREEN]);
  b -= GREEN;
  TRACE(b[GREEN]);
  b += RED;
  b.dump();
  b += YELLOW;
  b.dump();
  b += 47;
  b.dump();
  b += 48;
  b.dump();
  b += 100;
  b.dump();
  return (0);
}

This is the output:

b[GREEN] = 0
b[GREEN] = 1
b[GREEN] = 0
000100000000000000000000000000000000000000000000
001100000000000000000000000000000000000000000000
001100000000000000000000000000000000000000000001
001100000000000000000000000000000000000000000001
001100000000000000000000000000000000000000000001

Cheers!

Dani Maybe it is a good idea to change the title of the first post. 15->31 Best regards Jantje

After some testing it became clear that changing 1UL into 1ULL is enough to support 64 bit. without affecting 32 bit performance.

#define bitSet64(value, bit) ((value) |= (1ULL << (bit )) )
#define bitClear64(value, bit) ((value) &= ~(1ULL <<(bit)))

more about tests, see - https://github.com/arduino/Arduino/issues/1670 -

Jantje:
Dani
Maybe it is a good idea to change the title of the first post. 15->31
Best regards
Jantje

I did - hope it helps others.
Thanks again,
Dani