I am using Keil, C51 Compiler with Infineon XC886 8-bit micro....
I have a simple application that acquires rising and falling edges at P3.0 and then calculates the Period and Pulse Width in ticks of T12.
Then the program "based on these values" stretches the pulse using a 2D lookup table and generates a PWM output on P3.7 (it is inverted).
The lookup table returns a value from 0 to 1000.... a value of 300 means add 30% to the actual pulse width (NOT duty cycle which is %).
Everything seems to work except the output pulse width is incorrect.
I have (by process of elimintation) worked out the portion of code that is causing problems, but I think that it is actually the compiler.
The name of the function is "modifyDutyCycle"..
Step 1: First I disabled all interrupts then cut and paste the contents of the function into main. Since the calculation was quite involved I created a number of temporary variables to see the step by step results....
for (periodTemp = 3500; periodTemp < 31000; periodTemp += 200) { for(pulseTemp = 150; pulseTemp < 4000; pulseTemp += 50) { //Checking the lookUpTable function here in the for loop.... works! //index1 = lookUpTable(periodTemp, pulseTemp); //delay(10); //Extended version of the calculation from the "modifyDutyCycle" function //adjustVal = (uword)(((ulong)dutyCycle[0])*((ulong)(lookUpTable(period[0], dutyCycle[0])))/1000); //index2 = (uword)(((ulong)pulseTemp)*((ulong)(lookUpTable(periodTemp, pulseTemp)))/1000); index2 = lookUpTable(periodTemp, pulseTemp); //index2 = uword index3 = (ulong)index2; //index3 = ulong index4 = (ulong)pulseTemp; //index4 = ulong index5 = index3*index4; //index5 = ulong index5 /= 1000; //Attempt #1 //index6 = (uword)index5; //index6 = uword //Attempt #2 //blah = index5; //blah = uword //Attempt #3 index6 = index5; } }
Inititally "Attempt #1 failed to give the correct value" Then I just added yet another variable "blah" - which generated the correct value. Then I tried with no type cast with success
Step 2: I returned to the original function and tried to get it to work - still using temporary variables. When I simulate I get the following results...
1. There is no green/grey block on the side of the window (where you normally blace a breakpoint) at the "adjustVal = index4;" line
2. When I put the variable in the "watch" window however it seems to update the value of adjustVal
void modifyDutyCycle(void) { //sam uword index1; ulong index2; ulong index3; ulong index4; uword adjustVal; //sam #ifdef debug programStatus = 0x40; #endif dutyCycleNew[3] = dutyCycleNew[2]; dutyCycleNew[2] = dutyCycleNew[1]; dutyCycleNew[1] = dutyCycleNew[0]; //Calculate the extra number of timer ticks to be added to the output pulse //Stretch based on pulse width (off-time) not on total period //adjustVal = (uword)(((ulong)dutyCycle[0])*((ulong)(lookUpTable(period[0], dutyCycle[0])))/1000); index1 = lookUpTable(period[0], dutyCycle[0]); index2 = (ulong)index1; index3 = (ulong)dutyCycle[0]; index4 = index2*index3; index4 /= 1000; //adjustVal = (uword)index4; //typecast.. automatic?? adjustVal = index4; //index1 = index4; //dutyCycleNew[0] = dutyCycle[0]; // //Check that the new duty cycle is <100% if ((dutyCycle[0] + adjustVal) < (period[0] - 10)) { //Return new updated duty cycle dutyCycleNew[0] = dutyCycle[0] + adjustVal; } else //Error has occurred.... { dutyCycleNew[0] = period[0] - 10; //Make the pulse width to approx. 100% } }
3. When I try it on my hardware, I have a pulse output but the width is incorrect - which looks very similar to the first time I tried the funtion in main "Attempt #1"
My problem is something to do with a type cast or a memory addressing issue I think. But my results are inconsistent.
Any help on this oune would be much appreciated!!
Thanks, bye!!
" ... where you normally blace a breakpoint"
That means there's no code there on which to break; ie, it has been optimised away.
Note that you haven't shown your definitions of 'uword', 'ulong', etc.
Note also that 'word' as a type name is unhelpful - as the meaning of "word" is ambiguous.
"...the meaning of "word" is ambiguous."
When reviewing code, it's always best to start with the simple variable types. What is ambiguous for one can be blatantly obvious to another.
Here's one from my collection of the worst I've seen:
#define sl32 char
And no, there was no comment whatsoever.
That is interesting...
I am using the DAvE tool to generate my project. This is provided directly by Infineon.
Inside one of the header files it contains:
#define bool bit #define ubyte unsigned char #define uword unsigned int #define ulong unsigned long
If you say that this is bad coding, I would have thought better than that from the actual supplier of our micro!!
Good to have a data type for a signled 132-bit number.
If char can handle that, I'm a bit scared to ask for the size of int or long long int ;)
I said it was "ambiguous" - I'm glad you agree that's a "bad" thing!
"I would have thought better than that from the actual supplier of our micro!!"
Remember that they are chip manufacturers - not software developers. Unfortunately, "freebie" code from chip manufacturers is notoriously poor!
:-(
See rant here: www.8052.com/.../174319
There are, in fact, two issues with the code that you show:
1. They should be using typedef rather than #define;
2. As already noted, 'word' is ambiguous.
There is also a 3rd issue: The standard headers stdint.h and stdbool.h define a standard set of "portable" types - so they really shouldn't be re-inventing this wheel. However, I guess they will probably claim that this is just a legacy from before those standards...?
en.wikipedia.org/.../Stdint.h en.wikipedia.org/.../Stdbool.h
Maybe the meaning was intended to be, "a signed value with magnitude up to 132" - although that still wouldn't fit in a char...!
That's an ell not a one! Originally, I assumed it was a short form for 32 bit signed long.
"... although that still wouldn't fit in a char...!
A rather moot point ... technically, 131 might fit into a char - If the implementation took a char to be unsigned.
"A rather moot point ... technically, 131 might fit into a char - If the implementation took a char to be unsigned."
Technically, there isn't a size limitation on a char. You could have a 9-bit char. Or a char storing +/- 10000 decimal. And sizeof() would then return sizes of other data types in units of this char.
In the end, it's just happen to be common to have 8-bit signed or unsigned chars, which happens to work well with processors that have 16 or 32-bit register sizes.
There really is a good reason why programs should stay away from the "standard" data types char, short, int, long, ... unless it really is important what size they have. After all, the C standard does claim some minimum requirements for them.
Did you mean, un-important?
You are right. Should have stood "...] unless it really is unimportant [...]".
The C types are only fine when we really don't care, and the C type has a guaranteed range at least as large as what we need.
For all other situations, we should use custom data types. Either the stdint-defined types directly, or the project should contain custom definitions - and the build process should contain some tests to verify that the types really do fulfill the requirements.
No, if it is important, then the types from stdint should be used. Or the project should create own types that are explicitly verified that they have the exact size for that specific compiler.
Size char, short, int, ... should only be used where the real size is unimportant.
A loop [0..1000) will always fit in an int, so the real type isn't important - int will do fine. Besides that, people must remember that sizeof() isn't defined to return number of 8-bit quantities.
Back to the topic. I agree with Andrew's comment about code being optimized away. If you want to see intermediate calculation results, set the compiler optimization level to minimum or capture intermediate results in a way that does not depend on optimization level (printf?). Another point: apparently, your code is supposed to be fast. But the long multiplication and division (among other things) could take a while to execute. Are you sure you are not violating timing constraints that might be imposed by your application?
Sometimes, it can be good to scale numbers so the division instead will be 1024 or another two-potence. Division of numbers larger than the native register size is no fun thing. And depending on algorithm, the division may vary greatly in execution time depending on the specific values. This can create big problems for real-time-critical code.
what is bad about it? That the OP (and many other posters) should include the definitions in their post is, of course, a requirement
I have yet to see a place that do not "use abbreviations for variable types"
Erik
The whole point of having type definitions like this is to make the size & signed-ness certain.
The Bad Thing is that the meaning of "word" is ambiguous and, therefore, the meaning of uword is ambiguous.
the meaning of 'horse' is ambigous if you do not speak english, if you do, it is not.
the meaning of 'word' is ambigous if you do not work with the group that uses it, if you do, it is not.
I personally prefer US or U16, but last place I worked used 'u_word', and where I am now they use 'WORD', neither is ambigous to me, after seeing the dfinitions/standard sheet.