Any better way to reorder an out of order nibble in a AVR port?

Go To Last Post
23 posts / 0 new
Author
Message
#1
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

In order to avoid jumpers in a single layer PCB, I routed the output nibble from a DTMF decoder IC out of order to an AVR. With my limited knowledge in C, I managed to make it work. But now my code is almost at the mcu limit and looks like this piece of code may be reduced if made in a clever way. Any suggestions welcomed.

Thank you!

 

void CONVERT_DTMF (void){
    if (dtmf_original==0b1000){dtmf_reversed=0b0001; }        // 1
    else if (dtmf_original==0b0100){dtmf_reversed=0b0010;}    // 2
    else if (dtmf_original==0b1100){dtmf_reversed=0b0011;}    // 3
    else if (dtmf_original==0b0010){dtmf_reversed=0b0100;}    // 4
    else if (dtmf_original==0b1010){dtmf_reversed=0b0101;}    // 5
    else if (dtmf_original==0b0110){dtmf_reversed=0b0110;}    // 6
    else if (dtmf_original==0b1110){dtmf_reversed=0b0111;}    // 7
    else if (dtmf_original==0b0001){dtmf_reversed=0b1000;}    // 8
    else if (dtmf_original==0b1001){dtmf_reversed=0b1001;}    // 9
    else if (dtmf_original==0b0101){dtmf_reversed=0b0000;}    // 0
    else if (dtmf_original==0b1101){dtmf_reversed=0b1101;}    // *
    else if (dtmf_original==0b0011){dtmf_reversed=0b1100;}    // #
}

 

This topic has a solution.

Good Soldering JRGandara

Last Edited: Wed. Feb 13, 2019 - 04:53 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

for readability use CASE/SWITCH.  For speed if needed (I doubt it) use a sixteen value array in flash.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Well, this looks like more than just rearranging!

 

else if (dtmf_original==0b0101){dtmf_reversed=0b0000;}    // 0

 

Why not use a lookup table---then there's no truckload of conditional checks---contains only the values you need (plus some code that finds the data address).

 

 

By the way, since this is a decoder, this data is incoming to the AVR.... you might ignore the rearrangement....for example if you

had rcv code 2,2,3===>turn on pump   after scrambling would simply become:   rcv code 4,4, 12===>turn on pump

You only need to convert, if you want to display the values or show them to a person.

 

But now my code is almost at the mcu limit

What do you mean & why?  Is smoke pouring out of portb?

 

When in the dark remember-the future looks brighter than ever.   I look forward to being able to predict the future!

Last Edited: Fri. Feb 1, 2019 - 08:05 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

avrcandies wrote:

 

But now my code is almost at the mcu limit

What do you mean & why?  Is smoke pouring out of portb?

 

 

My guess is that they are running out of flash space.

 

Someone that needs to ask about a switch/case statement probably has lots of other weird things in the code taking up a lot of space.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Here's a macro for reversing a whole byte - https://www.avrfreaks.net/comment/204080#comment-204080.  You can skip the nybble swap if that's not needed.

 

.02

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Do you not read AVRfreaks ?

This thread is right up up there on page-1.

https://www.avrfreaks.net/forum/reverse-bit-order-byte

 

This is surely a bug.

    else if (dtmf_original==0b0101){dtmf_reversed=0b0000;}    // 0

 

Last Edited: Fri. Feb 1, 2019 - 08:22 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

from an other resent thread:

 

https://www.avrfreaks.net/commen...

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

It's pretty obfuscated because of the binary notation but the binary mbers on the right are just the same as the comment.

 

 

To OP:

What AVR are you using? you might be able to shave a few bytes off and the smallest code is probably a 16 byte array lookup table.

something like:

uint8_t convert_Dtmf ( uint8_t in){
  const static uint8_t lookup [16] = {    6,4,2,5,3,9,7, /* More numbers. */ };
 
 return lookup[ in & 0x0f];  // Read from inside out: 1). Take the lowest 4 bits of "in" 2). Use it as index in the array 3). return it.
}

 

To play nice you should also put the lookup table in flash, but the prefered syntax for that changed some time ago, and I forgot the details.

 

 

There is also nothing wrong with wire jumpers, and a few well placed jumpers would have been much easier than this bit fiddling.

 

 

 

 

Doing magic with a USD 7 Logic Analyser: https://www.avrfreaks.net/comment/2421756#comment-2421756

Bunch of old projects with AVR's: http://www.hoevendesign.com

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

If size not speed is your problem don't use a lookup table, it's way to small, it will take more code to set the pointers up than it save.

 

either a small loop or take a look at :

 

__builtin_avr_insert_bits

if you use GCC (my link in #7)

my guess it will be 10 instructions (1 clear, 4 load T , 4 save T one move to correct register)

 

 

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Not sure I understand the argument against a lookup table. That cascade of if() in #1 (or the alternative of switch/case) has got to be 30+ bytes at least. 

 

The highest input there is 0x0E so a 14 byte table will be enough. That leaves at least 16 bytes for the indexing. I can't believe it will take that many bytes for that.

 

The resulting code will be short and obvious to the reader.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

 

this code:

 

      a = __builtin_avr_insert_bits ( 0xFFFF0123, PINB, 0);
  dc:    83 b1           in    r24, 0x03    ; 3
  de:    90 e0           ldi    r25, 0x00    ; 0
  e0:    80 fb           bst    r24, 0
  e2:    93 f9           bld    r25, 3
  e4:    81 fb           bst    r24, 1
  e6:    92 f9           bld    r25, 2
  e8:    82 fb           bst    r24, 2
  ea:    91 f9           bld    r25, 1
  ec:    83 fb           bst    r24, 3
  ee:    90 f9           bld    r25, 0
  f0:    90 93 00 01     sts    0x0100, r25    ; 0x800100 <_edata>
 

is in reality 9 instructions (18 byte) for bit swap

I don't think that you can get the compiler (in ASM not a problem) to make code smaller than that (add: with a LUT). My comment was not about OP's org. code

Last Edited: Sat. Feb 2, 2019 - 03:11 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

clawson wrote:
The highest input there is 0x0E so a 14 byte table will be enough.
But that would need something like:


if( input > 0x0e) {
    return 0;
}

return lookup [BlaBla];

and the compare might as well be more than 16 bytes plus a simple and mask.

 

But there is not much to gain in such a small snippet as this.

OP probably has kilobytes of code in his program, and that is why I asked what AVR he's using.

 

I do not like the __builtin_avr_insert_bits much, A lookup table is very simple and straightforward. I cant figure out what that "insert bits" thing does without studying the manual.

It might be just me, but I like simple and straight forward code.

 

Doing magic with a USD 7 Logic Analyser: https://www.avrfreaks.net/comment/2421756#comment-2421756

Bunch of old projects with AVR's: http://www.hoevendesign.com

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

I'm losing track a bit, but how much does four out-of-=order bits really matter in real life?

 

I pose a hypothetical case...  four I/O bits on the same port.  >>IN<< order.

 

Best case scenario:  Together; in order; low four bits of port.  IN and mask.  Two cycles plus perhaps clearing room in a high register for the ANDI.

 

Now, regardless of how high in the air your nose is sniffing at the hardware designer, perhaps you shouldn't fire that person just yet.  Consider e.g. Mega88-family and port D.  Really hard to live there if you use the UART...

Now consider even if you have the bits together, there might be shifting involved.  An operation not painless on an AVR.  Then, I guess, we get to the "out of order".  The same overhead as scattered bits on various arbitrary ports and pins.  So you clear a register.  Then you do four SBIC/SBR pairs.  Eight cycles, right?  So instead of a couple/few cycles for the best case, now it might take a microsecond more.

 

I guess I'd need to agree with the nose-pointing if that read is SO important to the app and done so often that a microsecond is critical -- then do the layout for optimal access.  IMO/IME that situation would be quite rare.

You can put lipstick on a pig, but it is still a pig.

I've never met a pig I didn't like, as long as you have some salt and pepper.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

theusch wrote:
I'm losing track a bit, but how much does four out-of-=order bits really matter in real life?

Err ... If we believe OPs variable name it's DTMF so highly important. He can't send the backward bits to his DTMF library, the wrong codes will be generated.

I suppose the OP could reorganise the buttons on his keypad to suit his wiring mistake, then he wouldn't have to write any code. devil

 

 

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

I routed the output nibble from a DTMF decoder

Depending on the need, perhaps no unscrambling is needed (none needed for decisions/compares, just use the scrambled settings)....unscrambling might only be required for purposes, such as display. 

 

Someone was averaging 24 readings & wanting to know if the avg was more than 30...I asked why waste all that time, code space, and messing around, dividing by 24 (since they were also having trouble getting the division to work properly)

...just check if the sum is >720.   I got a blank stare.  "With averaging you have to divide by the number of samples!!!"   I guess I wans't familiar enough with how averaging works!

When in the dark remember-the future looks brighter than ever.   I look forward to being able to predict the future!

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

N.Winterbottom wrote:
Err ... If we believe OPs variable name it's DTMF so highly important. He can't send the backward bits to his DTMF library, the wrong codes will be generated.

Sorry; perhaps I didn't make the point clear in that lead-in.  The further analysis says it is roughly a microsecond for the straightforward unscrambling.  As mentioned, tell me when that might be of critical importance in real app.  Further, there may be a number of reasons that the routing isn't to the ideal place to make life easiest for the coder.

You can put lipstick on a pig, but it is still a pig.

I've never met a pig I didn't like, as long as you have some salt and pepper.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Thank you all for great suggestions! 

 

I´m not a real programmer. From time to time I end up having to write some code and when I get stuck in something, this AVR Freaks is an incomparable source of information and help. I could say that I´m able to solve 95% of my code problems with the forum search engine. But sometimes I´m not able to search for the right terms and end up asking something that already is answered. Sorry!

 

I tried the lut way and the code was reduced by more than 20%:
 

Remap_DTMF (uint8_t out_of_order){
	unsigned char lut[12] = {0x05,0x08,0x04,0x0c,0x02,0x0a,0x06,0x0e,0x01,0x09,0x0d,0x03};
	for (i=0; i<13;i++){
		if (out_of_order==lut[i]){
			return i;
		}
	}
}

This week I will try the other suggestions too. 

Ps: I can´t fire the PCB designer b/c I would end up without the inexperienced code guy (me) in the process. Some will think this would be a great idea, I know.

Good Soldering JRGandara

Last Edited: Sun. Feb 3, 2019 - 02:25 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

I´m using ATTINY2313 (2KB). The software was almost reaching the limit but is done. But I will review all the code and try to improve with the lessons of AVR Freaks. Of course if someone experienced do this same code will make in half and will laugh at me.

More about what the code has to do:

I get a nibble coming from a MT8870 DTMF Decoder. After a 3 digit password, the system will stay open until 25s pass without any DTMF detected. With the system open, there are several commands, like two digits ones for command on /off/pulse 8 relays. The states are saved in EEPROM in case of loss of power. There are commands with three digits for configurations and other longer for changing the password. I managed to do this code in about 6 hours. Speed is not a problem at all. Everything looks functioning as it should. But I´m still have to test for operations errors. This is only a remote control for personal use.

Good Soldering JRGandara

Last Edited: Sun. Feb 3, 2019 - 02:35 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

I tried the lut way and the code was reduced by more than 20%:

That's a 'reverse' LUT i.e. you're searching a LUT.

 

And surely that's not the real function.  Variable 'i' isn't declared, and the function has no return type.

 

You're also accessing beyond the array bounds (array is only 0-11, you're checking 12 and 13 as well).  Looks like you're missing '7' and '11'.  What gives?  DTMF supports 4x4 keypads for 16 buttons, but often only 3x4 for 12 buttons are used.  You're using 14?

 

A proper, straight-forward LUT:

int8_t Remap_DTMF (uint8_t out_of_order) {

  int8_t result = -1;

  static const __flash uint8_t lut[] =
    { 0x8, 0x4, 0xB, 0x2, 0x0, 0x6, -1, 0x1, 0x9, 0x5, -1, 0x3, 0xA, 0x7, 0xC, 0xD, };

  if (out_of_order < (sizeof(lut)/sizeof(*lut)) {
    result = lut[out_of_order];
  }

  return result;

}

This also puts the LUT in flash instead of SRAM.

 

An out-of-bounds value will return a negative number, so you can do a sanity check.  If that check is performed elsewhere, you can skip it in the function (but I wouldn't).

"Experience is what enables you to recognise a mistake the second time you make it."

"Good judgement comes from experience.  Experience comes from bad judgement."

"Wisdom is always wont to arrive late, and to be a little approximate on first possession."

"When you hear hoofbeats, think horses, not unicorns."

"Fast.  Cheap.  Good.  Pick two."

"We see a lot of arses on handlebars around here." - [J Ekdahl]

 

Last Edited: Sun. Feb 3, 2019 - 06:34 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

I tried your code just adding a missing parenthesis and rearranging the numbers. Worked perfect reducing the code in 94 bytes! I´m only using numbers and # sign. Not using letters A, B, C D or *. Thank you!

int8_t Remap_DTMF (uint8_t out_of_order) {

	int8_t result = -1;

	static const __flash uint8_t lut[] =
	{ -1, 0x8, 0x4, 0xc, 0x2, 0x0, 0x6, -1, 0x1, 0x9, 0x5, -1, 0x3, 0xb, 0x7,};

	if (out_of_order < (sizeof(lut)/sizeof(*lut))) {
		result = lut[out_of_order];
	}

	return result;
}

 

Good Soldering JRGandara

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

gandara wrote:
missing parenthesis
Oops, just typed it in without testing.

 

gandara wrote:
Not using letters A, B, C D or *.
So 11 out of 16, then.

 

joeymorin wrote:
Looks like you're missing '7' and '11'.
Whoops forgot '0' and '15'... and apparently I need a refresher on hexadecimal... 0x0E is 14d, not 15d.

 

So with those corrections, the LUT >>should<< be (assuming your reverse-LUT in #17 is correct):

	static const __flash uint8_t lut[] =
	{ -1, 0x8, 0x4, 0xB, 0x2, 0x0, 0x6, -1, 0x1, 0x9, 0x5, -1, 0x3, 0xA, 0x7, -1 };

This differs from yours in two ways:

  1. lut[3] is 0xB, not 0xC
  2. lut[13] is 0xA, not 0xB

 

Also, lut[15] is -1, whereas your LUT omits that index.

 

Note also that, since your reverse-LUT in #17 had 12 elements, this LUT has 12 non-negative elements even though you're not using '*'.

"Experience is what enables you to recognise a mistake the second time you make it."

"Good judgement comes from experience.  Experience comes from bad judgement."

"Wisdom is always wont to arrive late, and to be a little approximate on first possession."

"When you hear hoofbeats, think horses, not unicorns."

"Fast.  Cheap.  Good.  Pick two."

"We see a lot of arses on handlebars around here." - [J Ekdahl]

 

Last Edited: Sun. Feb 3, 2019 - 06:32 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

How does is stand compared to  __builtin_avr_insert_bits   ?

 

 

 

so take your code in #20 it will just be :

 

result = __builtin_avr_insert_bits(0xFFFF0123,out_of_order,0)
This reply has been marked as the solution. 
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0
int8_t Remap_DTMF (uint8_t out_of_order) {
	
	int8_t result = -1;
	
	static const __flash uint8_t lut[] =
	{ -1, 0x8, 0x4, 0xc, 0x2, 0x0, 0x6, -1, 0x1, 0x9, 0x5, -1, 0x3, 0xb, 0x7,};

	if (out_of_order < (sizeof(lut)/sizeof(*lut))) {
		result = lut[out_of_order];
	}
	if (result==12){
		LINE_OFF;
		f_senha=0;
		LED_RED_ON;
	}
	return result;
}

This was the code which reduced most. Thank you all for this solution!

Good Soldering JRGandara