Jump to content

Strange Error again

Thomas

By Thomas
in MIOS programming (C)

 Share

Recommended Posts

Thomas MIDIbox Newbie

Thomas

Posted
Posted

Hallo!

I wrote something like:

----------------------- VERSION that works ---------------------

void switchLEDs(unsigned char pattern, unsigned char and){

  unsigned char i;

  unsigned char pos;

  for (i = 0; i < 8; i++){

      MIOS_DOUT_PinSet1(32);

      MIOS_DOUT_PinSet1(33);

      MIOS_DOUT_PinSet1(34);

      MIOS_DOUT_PinSet1(35);

      MIOS_DOUT_PinSet1(36);

      MIOS_DOUT_PinSet1(37);

  }

}

----------------------------------------------------------------

This works.

BUT: all trials to make the pin setting dynamically depending on i failed!

----------------------- VERSION 1 that does not work  ---------------------

void switchLEDs(unsigned char pattern, unsigned char and){

  unsigned char i;

  unsigned char pos;

  for (i = 0; i < 8; i++){

      MIOS_DOUT_PinSet1(32 + i);

  }

}

----------------------------------------------------------------

This do not work, even if I use the temporär variable pos to store the result

of 32 + i. I thougth: ok, if it is a compiler bug and MIOS_DOUT_PinSet1 needs a contant parameter I reprogrammed it:

----------------------- VERSION 2 that does not work  ---------------------

void switchLEDs(unsigned char pattern, unsigned char and){

  unsigned char i;

  unsigned char pos;

  MIOS_DOUT_PinSet1(37);  //see below!

  for (i = 0; i < 8; i++){

      switch(i){

        case 0: MIOS_DOUT_PinSet1(32); break;

        case 1: MIOS_DOUT_PinSet1(33); break;

        case 2: MIOS_DOUT_PinSet1(34); break;

        case 3: MIOS_DOUT_PinSet1(35); break;

        case 4: MIOS_DOUT_PinSet1(36); break;

        case 5: MIOS_DOUT_PinSet1(37); break;

      }

  }

}

----------------------------------------------------------------

This didn't work, too. No pin is set, except pin 37. This was to indicate the function itselfs is called. I tried another version, with ifs:

----------------------- VERSION that works ---------------------

void switchLEDs(unsigned char pattern, unsigned char and){

  unsigned char i;

  unsigned char pos;

  MIOS_DOUT_PinSet1(37);

  for (i = 0; i < 8; i++){

      if (i == 0) MIOS_DOUT_PinSet1(32);

      else if (i == 1) MIOS_DOUT_PinSet1(33);

      else if (i == 2) MIOS_DOUT_PinSet1(34);

      else if (i == 3) MIOS_DOUT_PinSet1(35);

      else if (i == 4) MIOS_DOUT_PinSet1(36);

      else if (i == 5) MIOS_DOUT_PinSet1(37);

  }

}

----------------------------------------------------------------

Even this did not work! Only LED 37 ligths.

If I look at the *.asm the call of MIOS_DOUT_PinSet1(constant) is always the same. Independently if there are switches or ifs around it. But only the "pure" constant method above works.

I have no ideas. The *.asm seem to be ok.

As you can guess, the parameter "pattern" is the bit-pattern which of the LEDs 32-37 shall lit. But this is future...even the primitive varants do not work.

I have no idea.

TK. MIDIbox Guru

TK.

Posted
Posted

Hi Thomas,

which SDCC version are you using? 2.6.0 sometimes behaves strange, 2.5.0 works ok - undefined snapshot versions (with immediate bugs) shouldn't be used.

What happens when you are adding following code to the switchLEDs function:


void switchLEDs(unsigned char pattern, unsigned char and){
   unsigned char i;
   unsigned char pos;

   MIOS_MIDI_BeginStream(); // for debugging
   MIOS_MIDI_TxBufferPut(0xf0);

   for (i = 0; i < 8; i++){
       MIOS_MIDI_TxBufferPut(32 + i); // for debugging
       MIOS_DOUT_PinSet1(32 + i);
       MIOS_MIDI_TxBufferPut(MIOS_DOUT_PinGet(32 + i)); // for debugging
   }

   MIOS_MIDI_TxBufferPut(0xf7); // for debugging
   MIOS_MIDI_EndStream();
}
[/code]
Best Regards, Thorsten.
Thomas MIDIbox Newbie

Thomas

Posted
  • Author
Posted

Hi Thorsten!

I'm using 2.5.0 #1020 (8.Mai 2005).

But I could solve the problem.

The main problem was that in reality the 8 in the for-statement was no a 8, but a defined constant.

for (i = 0; i < (NO_OF_BUTTONS - POS_OF_SPECIAL_LEDS); i++) ...

where

#define NO_OF_BUTTONS 40

#define POS_OF_SPECIAL_LEDS NO_OF_BUTTONS - 8

I forgot the brackets - changing it to:

#define POS_OF_SPECIAL_LEDS (NO_OF_BUTTONS - 8)

seems to work.

That my mistake.

BUT:

1. during debugging I changed it to "8" sometimes - no result.

2. The resulting *asm depend if there are only constants or a variable parameter in the for-block.

Assuming that in th upper case POS_OF_SPECIAL_LEDS is defined as NO_OF_BUTTONS so (NO_OF_BUTTONS - POS_OF_SPECIAL_LEDS) == 0.

I would assume the for-statement is executed never. As it is.

But it is executed 8 times if ther are only constants in the body. Seems that the preprocessor calculates the values differently, depending on the body.

I changed your examble so that every time a complete MIDI-controller valus was sent.

Summary:

VERSION 1:

------------- in Header file -------------

#define NO_OF_BUTTONS 40

#define POSOFSPECIALLEDS NO_OF_BUTTONS - 8

----------------------------------------

------------- in function file -------------

void switchLEDs(unsigned char pattern, unsigned char and){

  unsigned char i;

  unsigned char pos;

  MIOS_MIDI_BeginStream();

  MIOS_MIDI_TxBufferPut(0xbb);

  MIOS_MIDI_TxBufferPut(0x00);

  MIOS_MIDI_TxBufferPut(0x11);

  for (i = 0; i < (NO_OF_BUTTONS - POS_OF_SPECIAL_LEDS); i++){

    MIOS_MIDI_TxBufferPut(0xbb);

    MIOS_MIDI_TxBufferPut(32);                            //constant!

    MIOS_MIDI_TxBufferPut(0x22);

    MIOS_DOUT_PinSet1(32);                                //constant!

    MIOS_MIDI_TxBufferPut(0xbb);

    MIOS_MIDI_TxBufferPut(MIOS_DOUT_PinGet(32)); //constant!

    MIOS_MIDI_TxBufferPut(0x33);

  }

  MIOS_MIDI_TxBufferPut(0xbb);

  MIOS_MIDI_TxBufferPut(0x00);

  MIOS_MIDI_TxBufferPut(0x44);

  MIOS_MIDI_EndStream();

}

----------------------------------------

I got:

BB 00 11

BB 20 22...  90 times in sum!*

BB 01 33... 102 times in sum!*

BB 00 44

* the order is mismatched:

BB 20 22

BB 01 33

BB 20 22

BB 20 22

BB 01 33

BB 01 33

BB 01 33

BB 20 22

....

VERSION 2:

------------- in Header file -------------

#define NO_OF_BUTTONS 40

#define POSOFSPECIALLEDS NO_OF_BUTTONS - 8

----------------------------------------

------------- in function file -------------

void switchLEDs(unsigned char pattern, unsigned char and){

  unsigned char i;

  unsigned char pos;

  MIOS_MIDI_BeginStream();

  MIOS_MIDI_TxBufferPut(0xbb);

  MIOS_MIDI_TxBufferPut(0x00);

  MIOS_MIDI_TxBufferPut(0x11);

  for (i = 0; i < (NO_OF_BUTTONS - POS_OF_SPECIAL_LEDS); i++){

    MIOS_MIDI_TxBufferPut(0xbb);

    MIOS_MIDI_TxBufferPut(32+i);                           

    MIOS_MIDI_TxBufferPut(0x22);

    MIOS_DOUT_PinSet1(32+i);                               

    MIOS_MIDI_TxBufferPut(0xbb);

    MIOS_MIDI_TxBufferPut(MIOS_DOUT_PinGet(32+i));

    MIOS_MIDI_TxBufferPut(0x33);

  }

  MIOS_MIDI_TxBufferPut(0xbb);

  MIOS_MIDI_TxBufferPut(0x00);

  MIOS_MIDI_TxBufferPut(0x44);

  MIOS_MIDI_EndStream();

}

----------------------------------------

I only got: (as you would expect)

BB 00 11

BB 00 44

VERSION 3:  (The right one)

------------- in Header file -------------

#define NO_OF_BUTTONS 40

#define POSOFSPECIALLEDS (NO_OF_BUTTONS - 8)

----------------------------------------

------------- in function file -------------

void switchLEDs(unsigned char pattern, unsigned char and){

  unsigned char i;

  unsigned char pos;

  MIOS_MIDI_BeginStream();

  MIOS_MIDI_TxBufferPut(0xbb);

  MIOS_MIDI_TxBufferPut(0x00);

  MIOS_MIDI_TxBufferPut(0x11);

  for (i = 0; i < (NO_OF_BUTTONS - POS_OF_SPECIAL_LEDS); i++){

    MIOS_MIDI_TxBufferPut(0xbb);

    MIOS_MIDI_TxBufferPut(32+i);                           

    MIOS_MIDI_TxBufferPut(0x22);

    MIOS_DOUT_PinSet1(32+i);                               

    MIOS_MIDI_TxBufferPut(0xbb);

    MIOS_MIDI_TxBufferPut(MIOS_DOUT_PinGet(32+i));

    MIOS_MIDI_TxBufferPut(0x33);

  }

  MIOS_MIDI_TxBufferPut(0xbb);

  MIOS_MIDI_TxBufferPut(0x00);

  MIOS_MIDI_TxBufferPut(0x44);

  MIOS_MIDI_EndStream();

}

----------------------------------------

I got what I want...

BB 00 11

BB 20 22

BB 01 33

BB 21 22

BB 01 33

BB 22 22

BB 01 33

BB 23 22

BB 01 33

BB 24 22

BB 01 33

BB 25 22

BB 01 33

BB 26 22

BB 01 33

BB 27 22

BB 01 33

BB 00 44

Strange, isn't it?

TK. MIDIbox Guru

TK.

Posted
Posted

I don't find this so strange - i is an unsigned variable, it can never be < 0

Therefore the behaviour of:

for(i=0; i<0; ++i)

is undefined and heavily depends on the C compiler implementation (a good C compiler prints a warning in this case)

Best Regards, Thorsten.

P.S.: yes, those missing brackets are not uncommon, all of my students @work are making this error once during their practical year ;-)

Thomas MIDIbox Newbie

Thomas

Posted
  • Author
Posted

Hi Thorsten!

I find it strange.  :-\

is undefined and heavily depends on the C compiler implementation (a good C compiler prints a warning in this case)

Your right. But the warning always says something like this:

"test.cpp:8: warning: comparison is always false due to limited range of data typ

e" (gcc, g++)

I will check this with Visual C++ later.

I always thought this statement was clear (always false) and is not undefined.

P.S.: yes, those missing brackets are not uncommon, all of my students @work are making this error once during their practical year ;-)

Maybe. But I knew that there must be brackets. I simply forgot them because I normally program JAVA (at work) and C++ and Python (at Home). So I never use preprocessor-directives for constants anymore (I read my Scott Meyers well... ;)) .

And all other variants don't need brackets...

Thomas MIDIbox Newbie

Thomas

Posted
  • Author
Posted

Hi!

FYI: Weil mich das jetzt sehr interessiert hat und ich (auf die Schnelle) darüber nichts finden konnte, habe ich die Frage in den einschlägigen Newsgroups gestellt.

Einhellige Meinung: die Bedingung müsste immer falsch sein, die Schleife nie durchlaufen werden. SDCC macht also hier sein eigenes Ding. (was aber nicht so schlimm ist, da es eh kein ANSI-C ist. Nur man muss es halt wissen.) 

Create an account or sign in to comment

You need to be a member in order to leave a comment

Create an account

Sign up for a new account in our community. It's easy!

Register a new account

Sign in

Already have an account? Sign in here.

Sign In Now
 Share
Go to topic listing
×
×
  • Create New...