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.

Link to comment
Share on other sites
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.
Link to comment
Share on other sites
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?

Link to comment
Share on other sites
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 ;-)

Link to comment
Share on other sites
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...

Link to comment
Share on other sites
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.) 

Link to comment
Share on other sites

Join the conversation

You can post now and register later. If you have an account, sign in now to post with your account.

Guest
Reply to this topic...

×   Pasted as rich text.   Paste as plain text instead

  Only 75 emoji are allowed.

×   Your link has been automatically embedded.   Display as a link instead

×   Your previous content has been restored.   Clear editor

×   You cannot paste images directly. Upload or insert images from URL.

Loading...
×
 Share
Go to topic listing
×
×
  • Create New...