We are running a survey to help us improve the experience for all of our members. If you see the survey appear, please take the time to tell us about your experience if you can.
Hello experts, to avoid a division, I use integer arithmetics. The unsigned int x which is in the range of 2000 is first multiplied by a number s which is 2048 plus/minus 1000 and then shifted by 11 digits which should replace a division by 2048.
unsigned int near x; unsigned int s x = ( (unsigned long) x * (unsigned long) s ) >> 11 ;
First, I am pretty sure any compiler written today will convert var / 2048 to a proper right shift of 11 bits so let the compiler do the work for you. Second, only one term in an expression need be a larger type to convert the entire expression to that type. E.g.
x = (unsigned int) (x * s) / 2048UL;
Oh, one other thing. If you must bit-shift, C only guarantees that you will pull in zeros when left shifting. Right shifting is implementation defined. So, in your example, you should AND off the top 11 bits of the result before using it.
Hello, I tried this on my system and the results were fine. The assembly code is;
s is assigned to R5 x is assigned to R4 MOV R5, DPP2:s MOV R4, DPP2:x MULU R4, R5 ;x*s MOV R5, MDH MOV R4, MDL MOV R6, R5 SHR R4, #0x0B ;>>11 (32-bit) SHL R6, #0x05 OR R4, R6 MOV DPP2:x, R4
x = (unsigned int) (x * s) / 2048UL; Unfortunately the above won't work as the cast to unsigned int has higher precedence than the division. You need to use: x=(unsigned int)((x*s)/2048UL); I feel it is always better to use parentheses to make your intentions clearer even when not required.
Right shifting is not implementation defined if the operand is unsigned. In the signed case the implementor has the choice of either shifting in zero-bits or copies of the leftmost bit (usually the sign bit).
Guenther, The line looked ok to me. I wonder if the fact that one variable is explicitly set as a near (and I presume the other is a far) is the problem. Perhaps making all variables far would help.
x = (unsigned int) (x * s) / 2048UL; Unfortunately the above won't work as the cast to unsigned int has higher precedence than the division. You need to use: x=(unsigned int)((x*s)/2048UL); Arg! Thanks. My bad. I certainly meant what you wrote. I feel it is always better to use parentheses to make your intentions clearer even when not required. I don't. Extra parentheses can actually harm readability in some cases. My mistake would not have been helped by parentheses since I put them in the wrong spot to begin with. My rule is, if the operators are on different levels of the precedence graph, don't use parentheses, e.g. this should be obvious to all C programmers:
if (0 < a && a < 100) ...
if ((0 < a) && (a < 100))
Please let me amend my previous post. I do believe in using parentheses when required.
Extra parentheses can actually harm readability I really must disagree with this. The parentheses group expressions so that you can see at a glance the order of evaluation. C has something like 16 levels of operator precedence as well as a left or right associativity for each level. If parentheses really do make the code less readable then the expression is too complex and should be split up. Perhaps the most important point is this, however: parentheses show the programmers intention even if the code is incorrect. It helps a great deal when debugging code to understand what the chap was trying to do. Also, if you are in the habit of using parentheses to show the order of evaluation you are much less likely to make the sort of mistake shown in the above post. this should be obvious to all C programmers All C programmers start out with no knowledge whatsoever of C operator precedence. They gradually remember more of the precedence rules as time passes but even when they reach the stage of being seasoned professionals they still have to get the book out once in a while.
I guess we'll have to disagree then. My intent was actually mis-represented by my misplacment of the parens. so I don't see how they helped. I've yet to have a hard time understanding
y = SLOPE * x + INTERCEPT;
if (0 < a && a < 100)
y = (SLOPE * x) + INTERCEPT;
I think the later is easier to read. I don't. What can I say? Another point is, that C programs have audiences other than people making their living programming C. They are by definition not the audience then. If non-programmers need to understand code they should either ask an experience programmer to summarize or learn C. We are not C experts to write dumbed down code folks. We are C experts because we learn the language and continually strive to improve and augment our C skills. I don't believe in writing unclear, obfuscated code but I also do not believe in writing C for the lowest common denominator of programmers. How will they ever learn? I don't use baby words with my 3 year old and she has a far better vocabulary than her peers who get spoken to as babies. Do you really want a bunch of programmer's writing basic C code or do you want them to grow into the language? Where do you draw the line? No use of ?:, no use of +=, *=, -=, etc., no use of bit-fields? No use of switch blocks? Are these constructs simply to diffucult for a programmer to understand? Nay, I think not. End of rant.
We are not C experts to write dumbed down code folks. We are C experts because we learn the language and continually strive to improve and augment our C skills. We are C experts when we write efficient, functional code that is clear and comprehensible to not only ourselves but those who will have to work on our code afterwards. This is one of the most important skills as a professional programmer. I don't believe in writing unclear, obfuscated code but I also do not believe in writing C for the lowest common denominator of programmers. How will they ever learn? Oh come on. Every 8051 programmer is familiar with the names of the commonly used special function registers: PCON, SCON etc. That is what they are called in the data sheets. Keil have very sensibly declared these in the device specific header files. Every Keil 8051 programmer will be familiar with using these - so what do you do? You ignore the Keil header file and give your own declarations. Please don't tell me that aids clarity. The first thing I'd do asked to work on your code would be some global replacement! Do you really want a bunch of programmer's writing basic C code or do you want them to grow into the language? Where do you draw the line? No use of ?:, no use of +=, *=, -=, etc., no use of bit-fields? No use of switch blocks? Are these constructs simply to diffucult for a programmer to understand? Nay, I think not. I want a bunch of C programmers writing clear, working code that everybody understands. This discussion was about the use of parentheses to aid clarity and overcome the operator precedence rules. Nobody is suggesting not using basic C constructs, were suggesting adding code to clarify intention.
Oh come on. Every 8051 programmer is familiar with the names of the commonly used special function registers: PCON, SCON etc. That is what they are called in the data sheets. Keil have very sensibly declared these in the device specific header files. Every Keil 8051 programmer will be familiar with using these - so what do you do? You ignore the Keil header file and give your own declarations. Please don't tell me that aids clarity. The first thing I'd do asked to work on your code would be some global replacement! I only change the case to lower and add an 'r_' prefix to indicate the SFRs are not regular variables (e.g. r_pcon vs PCON). ALL CAPS was a bad idea for the SFRs since most programmers use all caps for manifest constants. Also, SFRs in a header file is somewhat like defining a bunch of global variables in a header file. True, hardware is actually globabl but the namespace to access it does not need to be. In my UART driver I only needed to type a few SRF and SBIT. There is also some semblance of attempting to limit scope by placing the SFR definitions in the .c file instead of in the .h file, admittedly a semblance. Do you find my code that hard to read? Easier than most? More difficult than most? I've yet to have anyone complain about my code either from a functional standpoint or a readability standpoint. This small change does add clarity since you get extra visual information that this r_pcon is somehow not the same as a regular variable and it won't be confused with a manifest constant. Besides, I work with many CPUs (MIPS, PowerPC, ARM, PIC, MSP430, 8051, and Z80) and I do not remember all the registers for all of these, the r_ prefix helps me and is consistent with my coding standards, which I'm sure you'd hate, located on my website (ignore the confidential markers). http://www.embeddedfw.com Regards. - Mark
I only change the case to lower and add an 'r_' prefix to indicate the SFRs are not regular variables (e.g. r_pcon vs PCON). ALL CAPS was a bad idea for the SFRs since most programmers use all caps for manifest constants. Indeed, I normally only use all caps for manifest constants, but when there is something of a standard in place I stick with it so that my code has a similar look to the majority of the available codebase. I gradually get used to it until it seems perfectly natural and presto! I can glance at third party code and read it easily, others can do the same with my code. Also, SFRs in a header file is somewhat like defining a bunch of global variables in a header file. True, hardware is actually globabl but the namespace to access it does not need to be. In my UART driver I only needed to type a few SRF and SBIT. There is also some semblance of attempting to limit scope by placing the SFR definitions in the .c file instead of in the .h file, admittedly a semblance. As you say the hardware is global. What then is the point of trying to restrict scope or namespace? Bear in mind that we're talking about code for an 8 bit microcontroller here rather than something like MS Office. Do you find my code that hard to read? Easier than most? More difficult than most? I've yet to have anyone complain about my code either from a functional standpoint or a readability standpoint. I would find your code easy to read if you stuck to standard C data types and 'standard' 8051 nomenclature. typedefs and enumerated types do have their uses but always have me flipping backwards and forwards to and from header files to find out what the actual data type and value is. This disturbs the flow when trying to follow a piece of code. Everybody seems to have different ideas about these things - uINT8, Uint8, uChar, U8, u8 etc - I know they're all obvious but I'd rather have to deal with remembering that the Crayfish Vector 22B has a 13 bit unsigned char that all that. This small change does add clarity since you get extra visual information that this r_pcon is somehow not the same as a regular variable and it won't be confused with a manifest constant. It doesn't add clarity to the bulk of the 8051 community: those who are used to PCON, SCON etc. Besides, I work with many CPUs (MIPS, PowerPC, ARM, PIC, MSP430, 8051, and Z80) and I do not remember all the registers for all of these, the r_ prefix helps me and is consistent with my coding standards, which I'm sure you'd hate, located on my website. That's fine: if it helps you and you are going to be the maintainer of your code then do it. If you're writing 8051 code for the consumption of others I think you would be better sticking to the 'standard'. Your coding standards don't mesh perfectly with my preferences but you definitely are right about curly brace placement.