if V3<28V i want my PWM to start, and if V3>28V i want my pwm to stop, that's it, those are my 2 lines of code that i need for my charging. Is there a big problem with that? because everyone seems to not like that thing.

And if V3 is equal to 28? What then?

if (V3 < 28) {
pulsewidth=min(255,++pulsewidth;
} else {
pulsewidth = 0;
}

 

I really don't think this is a big issue. I'm sure he meant to write "if (pulseWidth == 0);". The Arduino compiler would not detect "if (pulseWidth = 0);" as a syntax error. "pulsewidth" would just become "0".
His "if" statements are a bit jumbled but we do not know what he is testing for so there is nothing we can do to help there.
Keith

No conforming compiler will report "if (pulseWidth = 0);" as a syntax error because it isn't. But most decent compilers will report a warning (depending on the warning level that is used) because it is such a common logic error. Unfortunately, far too many people ignore warnings entirely and consider the compilation successful since it did, after all, compile. This is why I recommended writing equality tests against constants with the constant on the left -- so that these common logic errors are turned into syntax errors that every compiler will catch, forcing them to be fixed.

 

if V3<28V i want my PWM to start, and if V3>28V i want my pwm to stop, that's it, those are my 2 lines of code that i need for my charging. Is there a big problem with that? because everyone seems to not like that thing.

What does it mean for your PWM to start and stop? Currently, you have that if V3<28 that the variable pulseWidth increases by one. How does that translate to "pwm to stop"? What happens if pulseWidth stays the same? What is the purpose of checking if pulseWidth is 255?

 

And if V3 is equal to 28? What then?

if (V3 < 28) {
pulsewidth=min(255,++pulsewidth;
} else {
pulsewidth = 0;
}

Ignoring the obvious typo of not including the closing paren on the call to min(), this invokes undefined behavior since pulsewidth is changed twice in the same statement (i.e., between the same pair of sequence points) and thus can be performed in any order.

Also, min() is not a standard C function and if it is implementation-defined then using arguments with side effects can be a really bad idea since it may be implemented as a macro. A common macro is

#define min(x,y) ( ((x)<(y))? (x) : (y) )

Which, in this case, would expand to

pulsewidth=( ((255)<(++pulsewidth))? (255) : (++pulsewidth) );

EDIT: fix typos.

 

If V3=27.99V my PWM wil start, from the first step to 255 every ms adding one more step, this is ok.
If V3=28V i don't care, because in the real world it will never be 28V for more than 1ms
If V3=28.01V the PWM is set to 0

i choose it to set to 0 immediately instead of "--" because of voltage overshoot in the case that i have a big load disconnecting from the batteries. This is it, these are the conditions i need for my charging. 28 is just a number for the test, most definitely it will change later on.

 

If V3=27.99V my PWM wil start, from the first step to 255 every ms adding one more step, this is ok.
If V3=28V i don't care, because in the real world it will never be 28V for more than 1ms
If V3=28.01V the PWM is set to 0

i choose it to set to 0 immediately instead of "--" because of voltage overshoot in the case that i have a big load disconnecting from the batteries. This is it, these are the conditions i need for my charging. 28 is just a number for the test, most definitely it will change later on.

So just do something like

if ( V3 < 28 )
{
   if (pulseWidth != 255)
      pulseWidth++;
}
else
   pulseWidth = 0;

NOTE: The first posting of this didn't have any curly braces and, as a result, I got bit by the classic dangling-else problem. Thanks to @xox for catching that and pointing it out.

If you like, you can add brackets to be safer in case more code is added to any of this later.

if ( V3 < 28 )
{
   if (pulseWidth != 255)
   {
      pulseWidth++;
   }
}
else
{
   pulseWidth = 0;
}

This most directly matches your logic.

You might be able to get some performance gain by leveraging the fact that any logical or relational expression evaluates to 0 if false and 1 if true, this you could simplify this to

pulseWidth = (V3 < 28) * ( pulseWidth + (pulseWidth != 255));

The concern here is that multiplication might be an expensive operation on your device. If that's the case, then this could be changed to

pulseWidth = (V3 < 28)? (pulseWidth + (pulseWidth != 255)) : 0;

Another option would be

pulseWidth += (V3 < 28)? (pulseWidth != 255) : (-pulseWidth);

A lot depends on how good your compiler is at optimizing and what capabilities your processor has. If your platform can do increments very well, then the full up if()-else code may well prove to be the fastest.

 

Be sure to understand why this is the case.

This claim in #6 is simply wrong; you might want to edit that post, as I did mine. In summary, try not to use assignment in "if" expressions, and always consider the warnings issued by the compiler.

Another minor quibble is that the floating point constants should be specified as single precision. Depending on default settings the compiler may very well obey your code as written and perform the calculation using double precision, which is likely not what you want, and may cause your executable to bloat as the otherwise unneeded libraries are linked in.

  V1 = (adc1 * 0.0001875) * 8; // double precision
  V1 = (adc1 * 0.0001875f) * 8; // single precision

 

This claim in #6 is simply wrong; you might want to edit that post, as I did mine.

I added a note to the post to clarify things.

Another minor quibble is that the floating point constants should be specified as single precision. Depending on default settings the compiler may very well obey your code as written and perform the calculation using double precision, which is likely not what you want, and may cause your executable to bloat as the otherwise unneeded libraries are linked in.

  V1 = (adc1 * 0.0001875) * 8; // double precision
  V1 = (adc1 * 0.0001875f) * 8; // single precision

A very good point for embedded applications.

Also, it might be better to keep to integer operations to begin with, although it will involve a division. If you do go that route, be sure that you don't have any possibility of overflow or underflow.

 

if ( V3 < 28 )
   if (pulseWidth != 255)
      pulseWidth++;
else
   pulseWidth = 0;

The brackets would actually be required there. That "else" statement binds to the second, rather than the first, "if" conditional.

It could however be restructured sans brackets:

if ( V3 >= 28 )
   pulseWidth = 0;
else if (pulseWidth != 255)
  pulseWidth++;

 

The brackets would actually be required there. That "else" statement binds to the second, rather than the first, "if" conditional.

It could however be restructured sans brackets:

if ( V3 >= 28 )
   pulseWidth = 0;
else if (pulseWidth != 255)
  pulseWidth++;

Yes, you are absolutely right -- the classic dangling-else bit me. I'll go back and update it.