Newbie needs help with arduino code

Posted by: @zeferby

bsolutely not ! These are the variables declarations+definitions that you need for the encoder interrupt routines,

Okay here is the new ino file with changes and the actual rpm is about 40 when  set it to be at 10, and about 10 rpm when I set it for 2 , just like before. Here is the strange thing, when the robot is turning the rpm calculates correctly!!

@chip I see you have kept lines 210-211 with the reset of lastencoderValue(L+R), which is still resetting those to 0 much too late :

When you come back to your next calculation of rpm(L+R), you get "under-estimated" values of encoder counts (missing some encoder steps because motors continue moving while your code executes between lines 149-150 and lines 210-211) => this will over-increase the speed

Also : I a wondering about encoderValueL/R usage and the cmmove variable ?

AND : are you sure this value is exactly for one revolution ?

#define ENC_COUNT_REV 374 

Posted by: @zeferby

Also : I a wondering about encoderValueL/R usage and the cmmove variable ?

  cmmoved=(encoderValueR / 10); 

this variable is to calculate how far chip (The Robot)  moved in centimeters. 
and I have removed the lines 210 and 211, and it now calculates correctly !! 
Thank you Sooooo much.

Phew !

Posted by: @zeferby

Phew !

lol , now maybe you could take a look at the way my logic works for turning and simplify that (big grin) 🙂

?

I guess i need to take a step back first...

Posted by: @zeferby

I guess i need to take a step back first...

LOL I don't blame you there because my not knowing how to program my, logic might not make perfect sense to anyone other than me :).

Thanks again for all your time and help.

No worry, I'll try to come back to this tomorrow

@chip

Without knowing exactly what this code is supposed to do, it does appear that there is an overlap between conditional statements, which appear to contradict one another?  Are you sure it really works as expected?

As the focus appears to be on heading, and you have multiple conditional statements in support try to counter that, you may want to restructure your algorithm and look into the "constrain(x, a, b)" function, there is a reference to it here:

constrain function reference

..which may help you to come up with a better algorithm.

Cheers!

Posted by: @frogandtoad

Without knowing exactly what this code is supposed to do, it does appear that there is an overlap between conditional statements

Thank you for your input and I should have explained what the code does, so let me try to explain my logic here. This code takes the value from the "compass (heading)" and will turn the robot to my "desired heading(Dheading)", the code will look at the difference between heading & Dheading and turn the robot in two different ways, if the difference is a small value it will speed up one of the motors for a short time then set the pwm value to that motor back to whatever it was before the turn. The second method of turning is for a large difference between heading & Dheading , this method turns the robot by rotating one wheel forward and the other wheel in reverse and it uses two variables(justshyofheading & justoverheading) to determine how much  delay value that will keep the motors spinning in opposite direction to achieve  the turn needed to get the "heading" value to match the Dheading value.

Posted by: @frogandtoad

Are you sure it really works as expected?

Well it works pretty well, it will align the robot to the desired heading and keep it on course and if it drifts off course by one degree it will re-align.

Posted by: @frogandtoad

As the focus appears to be on heading, and you have multiple conditional statements in support try to counter that, you may want to restructure your algorithm and look into the "constrain(x, a, b)" function, there is a reference to it here:

constrain function reference

It seems to me that function is for keeping a value within a specific range, and I don't understand how that applies to the code here, maybe you can reference the lines where this might be applicable.I am obviously not a programmer  and have relied on others to nudge me in right direction, I do appreciate the nudge.

Here is my ino file that may give more insight into my code as a whole. ANY input on how I might improve any part of it would be much appreciated.

@chip

I've had a quick look at your code, and here are some overall suggestions to clean it up a little:

The value of PI should be a constant, because it should never change, and you may want to access it from anywhere in your code, so it makes more sense to define it in the global scope of your program as well:

const float PI = 3.14159;

Rather than having many "if" conditional statements, creating a function with an appropriate name and combining common expressions is cleaner, and easier to understand the flow of the code.  Pre-increment and post-increment operators are also a good idea, as they tend to better document the intent of the action - I also removed the delay function, as I'm not really sure if it has any value:

void updateRPM() { 
  if (rpmR < 10 or rpmL < 10) {
     ++GOML; ++GOMR; 
    }
  if (rpmR > 10 or rpmL > 10) {
     --GOML; --GOMR; 
    }
 }

Now you can simply call the function as follows, in place of all the if statements:

updateRPM();

I haven't looked too close at your main "aligntocompass()" function yet, but as ZeFerby has also noted, it doesn't look quite right and contradictory as I originally noted.  As I am unable to test it, it makes it more difficult to try and refactor, but I'll try to understand it a little more and see if I can come up with anything suitable.

Btw, what are the explicit meanings for both Dheading and heading?
Also, where do some of these magic numbers come from, such as "6" in: justoverheading <= 6?

Posted by: @zeferby

: Select all with Ctrl+A, then right-click and "Auto Format"...

Thank you for that info, I had no idea how to do that 🙂

Here is my revised ino if you want to look it over, Thanks a million.