switch case or if else ?

[pdk5]

Hello,

I have to extract the info reg rank32 (5bits), rank16(4bits), rank8(3 bits), rank4(2bits), rank2(1bit), rank1(1bit) from the encoded 16 bits

// Extract the bits from the encoded rank infomation for each of the Antenna elements, as below.
// | 15 | 14 | 13 | 12 | 11 | 10 | 9 | 8 | 7 | 6 | 5 | 4 | 3 | 2 | 1 | 0 |
// --------------------------------------------------------------------------------------------------------------
// <------------ 32 elt -------------> <-------- 16 elt ------> <---- 8 elt ----> <- 4elt --> <-2elt-> <-NoData->
i.e if the last bit is 0, then its no data: rank32 = rank16=rank8=rank4=rank2=rank1=0
if the encoded info is : 1 then : rank32 = rank16=rank8=rank4=rank2=rank1=1
for other encoded info: then we extract the rank as follows:
rank1= 1
rank2 = Bit1 value +1
rank4= Bit 3 and 4 +1
rank8 = bit 6, 5, 4 +1
rank16 = bit 10, 9, 8, 7 +1
rank32 = bit 15, 14, 13, 12 +1

So I have written following two alternatives: could u comment which is better and any other comments please ?

	ASSERT_ONCE(usRank1elt == 0 && usRank2elt == 0 && usRank4elt == 0 && usRank8elt == 0 && usRank16elt == 0 && usRank32elt == 0);
	unsigned short usRank = usEncodedRank;

	switch (usRank)
	{
		case 0: break;
		case 0x01:
		{
			usRank1elt = 1; usRank2elt = 1; usRank4elt = 1; usRank8elt = 1; usRank16elt = 1; usRank32elt = 1;
		}
		break;

		default:
		{
			usRank1elt = 1;

			usRank = usRank >> 1;
			usRank2elt = (usRank & 0x01) + 1;

			usRank = usRank >> 1;
			usRank4elt = (usRank & 0x03) + 1;

			usRank = usRank >> 2;
			usRank8elt = (usRank & 0x07) + 1;

			usRank = usRank >> 3;
			usRank16elt = (usRank & 0x0f) + 1;

			usRank = usRank >> 4;
			usRank32elt = (usRank & 0x1f) + 1;
		}
		break;
	}

Other way is :

	ASSERT_ONCE(usRank1elt == 0 && usRank2elt == 0 && usRank4elt == 0 && usRank8elt == 0 && usRank16elt == 0 && usRank32elt == 0);
	unsigned short usRank = usEncodedRank;

	if (usRank & 0x01)
	{
		usRank1elt  = 1;
		usRank2elt  = 1;
		usRank4elt  = 1;
		usRank8elt  = 1;
		usRank16elt = 1;
		usRank32elt = 1;
	}
	else
	{
		usRank1elt = 1;

		usRank = usRank >> 1;
		usRank2elt = (usRank & 0x01) + 1;

		usRank = usRank >> 1;
		usRank4elt = (usRank & 0x03) + 1;

		usRank = usRank >> 2;
		usRank8elt = (usRank & 0x07) + 1;

		usRank = usRank >> 3;
		usRank16elt = (usRank & 0x0f) + 1;

		usRank = usRank >> 4;
		usRank32elt = (usRank & 0x1f) + 1;
	}

Thanks a lot
pdk

[VictorN]

Hello,

I have to extract the info reg rank32 (5bits), rank16(4bits), rank8(3 bits), rank4(2bits), rank2(1bit), rank1(1bit) from the encoded 16 bits

// Extract the bits from the encoded rank infomation for each of the Antenna elements, as below.
// | 15 | 14 | 13 | 12 | 11 | 10 | 9 | 8 | 7 | 6 | 5 | 4 | 3 | 2 | 1 | 0 |
// --------------------------------------------------------------------------------------------------------------
// <------------ 32 elt -------------> <-------- 16 elt ------> <---- 8 elt ----> <- 4elt --> <-2elt-> <-NoData->
i.e if the last bit is 0, then its no data: rank32 = rank16=rank8=rank4=rank2=rank1=0
if the encoded info is : 1 then : rank32 = rank16=rank8=rank4=rank2=rank1=1
for other encoded info: then we extract the rank as follows:
rank1= 1
rank2 = Bit1 value +1
rank4= Bit 3 and 4 +1
rank8 = bit 6, 5, 4 +1
rank16 = bit 10, 9, 8, 7 +1
rank32 = bit 15, 14, 13, 12 +1

So I have written following two alternatives: could u comment which is better and any other comments please ?

This one:

	switch (usRank)
	{
		case 0:
			break;

		case 0x01:
			usRank1elt = 1; 
			usRank2elt = 1; 
			usRank4elt = 1; 
			usRank8elt = 1; 
			usRank16elt = 1; 
			usRank32elt = 1;
			break;

		default:
			usRank1elt = 1;

			usRank = usRank >> 1;
			usRank2elt = (usRank & 0x01) + 1;

			usRank = usRank >> 1;
			usRank4elt = (usRank & 0x03) + 1;

			usRank = usRank >> 2;
			usRank8elt = (usRank & 0x07) + 1;

			usRank = usRank >> 3;
			usRank16elt = (usRank & 0x0f) + 1;

			usRank = usRank >> 4;
			usRank32elt = (usRank & 0x1f) + 1;

    		        break;
	}

[pdk5]

Thanks a lot for the comments and reviewing my code

[2kaud]

Note that the 2 versions of the code (if/switch) are not the same.

For the if statement, the test is usRank & 0x01 ie it is only testing 1 bit.

For the switch statement, the test is usRank == 0x01 ie it is testing that the whole of usRank (all the bits) is equal to 1 and not just one bit (all bits 0 except for bit 1). I don't think this is what you mean?

Also note that to set all the variables to 1, you can use:

usRank1elt  = usRank2elt  = usRank4elt  = usRank8elt  = usRank16elt = usRank32elt = 1;

For info. Do you know about bit fields? if not, look them up. See https://en.cppreference.com/w/cpp/language/bit_field

Using these could make coding much easier when dealing with specific bits in say a 16 bit variable.

Also, I would suggest using uint16 as the type, rather than unsigned short.

[pdk5]

Thanks a lot kaud comments and suggestion. Very helpful.

Yes i.e true, my if else will not work !!!

Was testing now and found that.


Yes, i need to test all 15 bits are 0 except the last bit as 1. i.e 0000 0000 0000 0001, then the usRank1elt = usRank2elt = usRank4elt = usRank8elt = usRank16elt = usRank32elt = 1;
But as you suggested, switch case works


Thankyou very much for all the help.

[pdk5]

I wrote a sample program using the bitfields:

#include "stdafx.h"

#include <iostream>
using namespace std;

struct S {

	unsigned short usRank32elt : 5;
	unsigned short usRank16elt : 4;
	unsigned short usRank8elt : 3;
	unsigned short usRank4elt : 2;
	unsigned short usRank2elt : 1;
	unsigned short usRank1elt : 1;
};

int main() {
	unsigned short usEncodedRank = 0x02;
	S s;
	*(reinterpret_cast<unsigned short *> (&s)) = usEncodedRank;

	cout << s.usRank1elt << "\t" << s.usRank2elt << "\t" << s.usRank4elt << "\t" << s.usRank8elt << "\t" << s.usRank16elt << "\t" << s.usRank32elt;
}

It is strange that usRank32elt value is 2. others are 0.

Looks like "endianness" matters here as more than one byte !!!

[pdk5]

Found a bug in my previous code: Modified one is :

	unsigned short usEncodedRank (0x02);
	unsigned short usRank1elt(0), usRank2elt(0), usRank4elt(0), usRank8elt(0), usRank16elt(0), usRank32elt(0);
	unsigned short usRank = usEncodedRank;

	// Only if the LSB is set, the encoding is valid
	if ((usEncodedRank & 1))
	{
		switch (usEncodedRank)
		{
		case 0x01:
			usRank1elt = usRank2elt = usRank4elt = usRank8elt = usRank16elt = usRank32elt = 1;
			break;

		default:
			usRank1elt = 1;

			usRank = usRank >> 1;
			usRank2elt = (usRank & 0x01) + 1;

			usRank = usRank >> 1;
			usRank4elt = (usRank & 0x03) + 1;

			usRank = usRank >> 2;
			usRank8elt = (usRank & 0x07) + 1;

			usRank = usRank >> 3;
			usRank16elt = (usRank & 0x0f) + 1;

			usRank = usRank >> 4;
			usRank32elt = (usRank & 0x1f) + 1;

			break;
		}
	}
	cout << usRank1elt << "\t" << usRank2elt << "\t" << usRank4elt << "\t" << usRank8elt << "\t" << usRank16elt << "\t" << usRank32elt;

[pdk5]

@kaud: Now i have a doubt, will endianess, affect my shifting as it is 2 bytes .

[2kaud]

@kaud: Now i have a doubt, will endianess, affect my shifting as it is 2 bytes .

Endiness matters a great deal when dealing with bit fields! Also, you don't need the cast stuff - just use a union. Consider:

#include <iostream>
#include <iomanip>

using namespace std;
using uint16 = unsigned __int16;

union Rank {
	uint16 rank;

	struct S {
		uint16 usRank1elt : 1;
		uint16 usRank2elt : 1;
		uint16 usRank4elt : 2;
		uint16 usRank8elt : 3;
		uint16 usRank16elt : 4;
		uint16 usRank32elt : 5;
	} s;
};

int main() {
	Rank r;

	r.rank = 0x03;

	cout << r.s.usRank1elt << "\t" << r.s.usRank2elt << "\t" << r.s.usRank4elt << "\t" << r.s.usRank8elt << "\t" << r.s.usRank16elt << "\t" << r.s.usRank32elt << endl;

	r.rank = 0x220;

	cout << r.s.usRank1elt << "\t" << r.s.usRank2elt << "\t" << r.s.usRank4elt << "\t" << r.s.usRank8elt << "\t" << r.s.usRank16elt << "\t" << r.s.usRank32elt << endl;;

	r.rank = 0;
	r.s.usRank16elt = 0x1;
	r.s.usRank32elt = 0x2;

	cout << hex << "0x" << setw(4) << setfill('0') << r.rank << endl;
}

which is for little endian for Intel cpus.

[pdk5]

Thanks a lot kaud.

Assuming it is little endian, then my code (the last post, with shift lefts) wont work !!! But when i tested it works !!

eg
int main() {

uint16 x = 0x00ff;

uint16 y = x >> 1;

cout << y;
}


eventhough x is stored in as bytes, lower address as 0xff, higher address as 0x00. When i try the x >>1 , i think compiler reads the value from memory and puts in right order and then only shifts.

i.e why i get value of y as 0x7f

i.e 0000 0000 1111 1111
to 0000 0000 0111 1111

So my code works

[2kaud]

shift left/right deals with the endianess assuming the original data was little endian. The bit fields for little endian are as my post #9.

The problem comes if you're getting data from an external source (eg the antenna data?). You need to know its endianess. If its little endian - OK. If it's big endian, then the layout would be as per your post #6.

When you're testing, are you using actual antenna data as obtained from the antenna (?) or just setting a variable to a value?

[pdk5]

Thankyou very much kaud. That data is provided by third party. I guess, in that case needs to discuss with them.

[pdk5]

As per post #10:

i.e uint16 num = 0x00ff;

0x00ff is stored as :
low address 1111 1111
high address : 0000 0000

right shift should have resultedd in : 0111 1111 1000 0000 (0x7f10)
But is resulting in : 0000 0000 0111 1111 (0x7f)

So my Q is compiler wont read the bitwise for bit operations from memory: but it considers the entire value ?
Am i wrong ? I am using windows 10 visual studio 2019

[VictorN]

As per post #10:
0x7f is stored as :
low address 1111 1111
high address : 0000 0000

No, 0x7f is (as binary) 0111 1111
binary 1111 1111 is 0xFF

[pdk5]

Sorry corrected it Victor. thanks