| Electronics > Beginners |
| Arduino Code - volts/amps/power monitor |
| << < (5/10) > >> |
| newbrain:
And now that we have perhaps sorted out the major things, let's go with the nitpicking :blah: @Rick Law: I'm not a fan of extra casts, as I prefer to make sure that the expression is correctly written in the first place. I treat them as a strong spice in a recipe: it's has its place, in moderation ;). --- Quote ---#define ADC_AMPS(adc,counts) ( (float)((float) adc)*26.7/( ((float)counts)*1023.0) ) --- End quote --- The guideline to put parenthesis around a function-like macro parameter is definitely sound and correct. Unfortunately, the code above does not do that! Can you spot the case where the (float) cast is not applied correctly to the #define parameter? It is, I admit, an uncommon, situation, but, it happens, and, is difficult, to spot. Hint in the period above. It does not make much difference here (conversion to float is guaranteed by all the other operands), but could in other contexts. @metrologist A couple of remarks, the first one is very general: We have till now talked about float constants, but there isn't any in all the posted code! :-// Am I kidding? Actually not, all the constants here are floating point, but their type is, according to 6.4.4.2 clause 4 actually double. For the Arduino (AVR+gcc) implementation, floats and doubles are in reality the same type, or to be more precise, have the same internal representation. In other environment using 100.0 instead of 100.0f can change the type of an expression and lead e.g. to different execution times (floats can be slower or faster than doubles). The second point is a specific value: 1023. The ATmega ADC specifications say, in chapter 24.7 of the 328 datasheet that: $$ADC = {V_{IN} \cdot 1024 \over V_{REF}} $$ and: --- Quote ---0x000 represents analog ground, and 0x3FF represents the selected reference voltage minus one LSB. --- End quote --- So, why divide by 1023 instead of 1024? * It's not correct according the DS for this ADC Note that other ADCs (e.g. LTC2400, Tabe 2) might use an all ones code to represent VREF * It could have performance impacts. More or less guaranteed when using integers, but a smart compiler can also optimize the FP division by a constant of the form 2n (Hint: FLT_RADIX) Given the relatively low precision of the ATmega ADC (2LSB, IIRC) this also is a moot point, but let an old man rant... :blah: PS: Picture from Kara no Kyoukai: Fukan Fuukei (The Garden of Sinners: Overlooking View), first instalment of a great anime movie series. Recommended if you like dark supernatural stories. Not for kids. |
| Rick Law:
--- Quote from: newbrain on March 10, 2018, 11:06:22 am ---And now that we have perhaps sorted out the major things, let's go with the nitpicking :blah: @Rick Law: I'm not a fan of extra casts, as I prefer to make sure that the expression is correctly written in the first place. I treat them as a strong spice in a recipe: it's has its place, in moderation ;). --- Quote ---#define ADC_AMPS(adc,counts) ( (float)((float) adc)*26.7/( ((float)counts)*1023.0) ) --- End quote --- The guideline to put parenthesis around a function-like macro parameter is definitely sound and correct. Unfortunately, the code above does not do that! Can you spot the case where the (float) cast is not applied correctly to the #define parameter? It is, I admit, an uncommon, situation, but, it happens, and, is difficult, to spot. Hint in the period above. It does not make much difference here (conversion to float is guaranteed by all the other operands), but could in other contexts. ... ... --- End quote --- Hmmm.... I cannot spot the problem why "(float) cast is not applied correctly". You have to enlighten me here. Except I did noticed while I say one should add parenthesis around the #define's arguments, I actually did not add that in the #define shown. So if a complex expression is used as an argument here, without the needed parenthesis, the -cast- could run into issues. That was careless of me saying one should do it and then omitted it in the final text. So, #define ADC_AMPS(adc,counts) ( (float)((float) adc)*26.7/( ((float)counts)*1023.0) ) should really be: #define ADC_AMPS(adc,counts) ( (float)((float) (adc))*26.7/( ((float)(counts))*1023.0) ) for the original expression: (float)a*26.7/(count*1023.0) Is that what you are pointing out? If not, I really would like you to enlighten me as to what I missed. As to having the extra -cast- you and I have a disagreement. It is of course good to do it right the first time. But when code is maintained by unknown number of individuals and done a decade or more later, I assume some time down the road someone would miss something. This extra -cast- is like an extra layer of bubble-wrap just to add a bit more protection. It reminds me (or someone else) that there is a reliance on the argument being "of a specific type". God knows, if people are careful, no fool would talk about needing parenthesis and then didn't put the parenthesis in with the example... |
| newbrain:
I probably was not very clear (in trying to be cheeky), and also thought that you meant that ((float)param) was enough; after all, in the form shown, most integer expressions will work correctly and result in being cast to a float, as at least one operand will. I apologize for the misunderstanding, I hope you did not take offence. The corner case I was addressing is quite convoluted, and I would think it would normally not occur...but that's what sometimes break things :( If there's an object-like macro such as: --- Code: ---#define z x,y --- End code --- with a comma operator, and that's passed as parameter, the (float) cast will only be applied to the first operand. That operand is then thrown away (treated as a void expression by the comma operator), and the second one (without any cast) used in the final expression: the cast has not been applied at all to the object whose value we are interested in. The intermediate #define is needed, as it's the only way I was able to find to pass a comma expression without using parenthesis (otherwise the cast would apply) or messing up the parameter list of the function-like macro: as said, contrived and unlikely... As for the casts we can agree to disagree. IME, when I see old code with a lot of casts I always wonder whether they are there to shut up some warning, to hide some problem in type choices, or "make it work" without a full understanding of where the problem was. They may have been put there with good reason, by a reasoning coder, but they always generate a lot of head scratching... In our specific case, the whole define will be evaluated as double, as the used constants are double, and the first (float) applies to just ((float)(adc)): we need another pair of parenthesis. My next post will probably need to refer to a different standard >:D I think there is a good learning point from all this discussion on macros: Macros are a powerful instrument, but making them safe is difficult and sometimes impossible. Readability is bound to suffer. But rejoice, there's hope: For most function-like macros, C99 (and later) provides a more powerful instrument in the form of inline functions (6.7.4, clause 6): --- Code: ---static inline float ADC_AMPS(float adc, float amps) { return (adc*26.7f)/(counts*1024.0f); } --- End code --- Clear, concise, type safe, and it is as fast as the macro in decent compiler implementations. The comma trick does not work here. ::) |
| Rick Law:
--- Quote from: newbrain on March 11, 2018, 12:27:23 am ---I probably was not very clear (in trying to be cheeky), and also thought that you meant that ((float)param) was enough; after all, in the form shown, most integer expressions will work correctly and result in being cast to a float, as at least one operand will. I apologize for the misunderstanding, I hope you did not take offence. ... --- End quote --- No offense taken but instead a thank you! I was the fool who talked about one should add the parenthesis and then I missed it in the last step... So thanks for finding the error there! --- Quote from: newbrain on March 11, 2018, 12:27:23 am ---... I think there is a good learning point from all this discussion on macros: Macros are a powerful instrument, but making them safe is difficult and sometimes impossible. Readability is bound to suffer. But rejoice, there's hope: For most function-like macros, C99 (and later) provides a more powerful instrument in the form of inline functions (6.7.4, clause 6): --- Code: ---static inline float ADC_AMPS(float adc, float amps) { return (adc*26.7f)/(counts*1024.0f); } --- End code --- ... ... --- End quote --- re: "Macros are a powerful instrument, but making them safe is difficult" Yeah, you said it. But when they are simple ones, it makes life a lot easier. re: "inline functions" Back when I was doing more C coding (1980's), it wasn't available then. Now that I am doing coding just for play, they have the things I love to have then. There is so much more fun things like in-line functions. Even better is with appropriate option flag settings, the compiler will make them in-line even if you don't explicitly say so. So, these days, I let the compiler do that and use functions more liberally. |
| paulca:
floats are evil. double is double trouble. macros are often a curse for debugging, though can aid it if used right. Do not get carried away. If you have to read someone elses code and they are a macro-ist you will hate them. Nested, conditional, complex and parameterised macros can make debugging a nightmare and require that you end up running the pre-processor to resolve them to be able to find the bugs. For the love of God split your code up into functional chunks! For this kind of thing use "top down". Take your entry point and write, in english the steps you want it to do: readSensors totalize display Then stick brackets on them. readSensors(); totalize(); display(); Go write those functions. Repeat the pattern until you find you can only realistically replace the english with less than 3 or 4 lines of code. Write a test main() or loop() which tests those functions independently and outputs PASS or FAIL on Serial. This relates to globals.... which make independent testing of "separation of concern" almost impossible. Doing it "nicely" requires you split your code into different files. You can also use #ifdefs to remove test code from the final build. I'd be tempted to suggest you avoid globals as well, but as we are in uC land with small single file applications I suppose it won't hurt. A convention is to prefix them with "g", so everyone (including yourself in 6 weeks time) knows they are global. The alternative is structs (or classes) and that leads to memory management, side effect arguments and pointers which leads to bugs for new players. On data integrity of precision: * Always go up before coming down. Multiply before divide. * Use the highest units/denominations you can. * Remember that computers suck at maths. Try this: double a = 10 / 3; Serial.println( (a * 3) ); You won't get 10. More likely 9.999999999999.... Even (1/10.0)*10 will not give you the right answer unless the compiler spots the cancellation. In your case I would stay away from floating point altogether and work in millivolts, milliamps and milliwatts. For the energy I would use milli-Joules rather than Ah. Ah is a large value compared to it's equivalent in joules so you need higher precision for Ah than joules. Joules = power(watts) * time(seconds). 1Ah @ 1V = 3600J. Be careful though, 1 mV * 1mA * 1mS = 1 nano-joule. Calculating Ah in every iteration with a different voltage will potentially lead to non-sense. When you get to the end of your process, ask the question... how much energy was that in joules? The answer will always need you specify the volts.... which you now don't know. For a battery it's often just it's nominal voltage, but you didn't store it as such, so it's non-sense and you will never be able to actually calculate total energy. Storing watt hours or joules decouples you from the voltage. The difference is that Ah is a unit of energy at a given voltage. Watt hours and joules are units of actual energy. You can convert to Ah at the battery nominal voltage later. The only reason for using Ah is for a battery to say, "I will deliver X amps for Y hours at some voltage between my maximum and my minimum", it will not tell you total energy. I suppose it depends on what you want. For wrapping variables you can always use the modulus operator. wrappedCounter = wrappedCounter++ % MAX_VALUE; % gives the remainder after division. So once wrappedCounter reaches MAX_VALUE the modulus returns 0. Note the post increment not pre-increment or you would never get MAX_VALUE.... unless that's what you want. In fact, asides for data type rollover you don't even need to reassign the result of modulus for it to work. counter++; boundedValue = counter % MAX_VALUE; |
| Navigation |
| Message Index |
| Next page |
| Previous page |