Newbie needs help with arduino code

@chip

Ok, one small step : move that block into its own function called something like void calcRPMs(), and replace it with the call in void loop()

Then check everything works as before.

@chip

Another small step : as above, extract to their own function (like void updateDisplays() or something like that) all this block of lines from void loop(), and replace them with the call to the new function.

Block starting with this line : 

Up to that line :

After these two small "refactoring" steps, which are supposed to leave intact the functionalities of the program, you will have a much more readable void loop() code 

@zeferby

Posted by: @zeferby

Checking your chip2_11_17_2019.ino and the other code above, I have several suggestions 

I agree with all of your suggestions, except for one small note that "M_PI" is not an official part of the ISO language standard, for either C or C++.  Some compiler implementations do add it as an extension, but it's not something that is required to be implemented as part of the standard, thus cannot be 100% relied upon to be available.

The following check can be used to reduce any risk:

#ifndef M_PI
  #define M_PI 3.14159265
#endif

Having said that however , it is preferable to just declare it as a constant, then there is no need to rely on it being available or not, and you can move on with confidence:

const double PI(3.14159265);
  Or...
const double PI = 3.14159265;

Posted by: @zeferby

Generally, try to make one small change and test the results.

When you're satisfied : next small change+test

etc...

Totally agree... compile often, get the small chunks of code working before moving on.

@chip

I would also add that code should be appropriately documented... variable names, comments, etc... and any code that is not being used, should be removed before you post it, because it just makes it harder to look through it all, only to find out that it doesn't do anything, for example:

The "dow2String" function is not being used, but you force us to look for it's use in the code, only to find out it has been commented out.  Likewise, there is a "duration1" and  "distance1", implying there is other distance and duration's, which there is, but are both never used either.  The following made me smile:

Posted by: @zeferby

After these two small "refactoring" steps, which are supposed to leave intact the functionalities of the program, you will have a much more readable void loop() code 

I have made the changes you suggested and everything seems to be working just fine 🙂

Thank you again for all your time and effort Eric !!

here is my new ino if you want to look it over and see if i did it correctly.

Posted by: @frogandtoad

I would also add that code should be appropriately documented... variable names, comments, etc... and any code that is not being used, should be removed before you post it, because it just makes it harder to look through it all, only to find out that it doesn't do anything,

Thank you for your suggestions and I have added a lot of comments and variable name comments so maybe it will be easier for others to understand my illogical mind 🙂

There are still some variables not being used in the code, it is getting late here and the wife is telling me to get off here so I best get lol , See ya's

@chip

No worries... I'll download your .ino file, and have a look at it again with respect to clarity, etc...  I just don't have a compass to play with to experiment, but I'll see if I can get one soon.

BTW, what is this code supposed to do?

byte address;
void debug(byte address) {
  display.print("Found at 0x");
  display.println(address, HEX);
 }

...as further down in the code you set it to zero twice, which doesn't appear to change anything?

  if (heading == 0) {
    address = 0; // --------------HERE
    scanner.Init();
    display.clearDisplay();
    display.setCursor(0, 0);
    address = 0; // --------------HERE
    scanner.Execute(debug);

Posted by: @frogandtoad

BTW, what is this code supposed to do?

byte address;
void debug(byte address) {
  display.print("Found at 0x");
  display.println(address, HEX);
 }

...as further down in the code you set it to zero twice, which doesn't appear to change anything?

  if (heading == 0) {
    address = 0; // --------------HERE
    scanner.Init();
    display.clearDisplay();
    display.setCursor(0, 0);
    address = 0; // --------------HERE
    scanner.Execute(debug);

First off thank you for your time to help me, so about the code above I have been trying to troubleshoot an issue where my compass will out of the blue at random times just report a heading of zero and this code will at that time show all the I2c devices "online" and display all the HEX addresses on the display for each device. The line of code ( addresses = 0) will clear any previous I2c addresses found and I do it twice just to be sure any devices reported are the only devices online during the current I2c bus scan. My thinking is maybe the compass is loosing communication with the Arduino Mega and if I get a reading of (heading = 0) maybe it went offline for just a moment. Although every time it has reported a heading of zero the I2c scan shows it and all other I2c devices to be online.

Posted by: @zeferby

i think that currently if you are headed to a 1deg "actual" and want to head to a 359deg "desired", your robot goes through a big 358deg turn tot he right instead of a small 2deg adjustment to the left ?

Yes you were correct, and that was the only way I knew how to do it :). Sorry I missed that question before now.

Posted by: @zeferby

• re-structure your code to use a "delta+direction" approach like the code you found on Github. For example something like :

#define TURN_THRESHOLD    6 //"hard turn" over 6 degrees heading delta
...
void alignToCompass()
{
  int headingDelta = (Dheading - heading); // headingDelta = 358 for Dheading = 359 and heading = 1
  if (headingDelta == 0) {
    return;
  }
  if (headingDelta > 180) { //true
    headingDelta = headingDelta - 360; // headingDelta = -2
  }
  if (headingDelta < -180) { //false
    headingDelta = headingDelta + 360; //not executed
  }
  
  bool bTurnRight = (headingDelta > 0); // false = turn left
  headingDelta = abs(headingDelta); // headingDelta = -2
  if (headingDelta) {// "hard turn"
  }
  else {// "soft turn"
  }
...
}

I have implemented the new alighToCompass() function you suggested and edited I have edited it somewhat , although I am not sure how to turn left/right, maybe you could nudge me in the rite direction here 🙂 .

Thanks as always for your help.

Here is my current code with the old align function commented out.

@chip

About the compass values, instead of discriminating against the innocent 0 heading, you probably should check if the sensor event read is valid : as seen on the sensor library code, getEvent returns a bool value that you maybe sometimes could catch as false.

https://github.com/adafruit/Adafruit_LSM303DLHC/blob/ccc4e70113460333a70926e08150d2d13a32c35b/Adafruit_LSM303_U.h#L181

https://github.com/adafruit/Adafruit_LSM303DLHC/blob/ccc4e70113460333a70926e08150d2d13a32c35b/Adafruit_LSM303_U.cpp#L435

League of North Heading Defenders, Declination Subdivision

@chip

 ? Bug-hunting exercise 1 !

Before going into the "Delta variant" of void aligntocompass(), try to use this code to find a bug in your original version. This is your original code with the "hardware stuff" replaced with debugging code. Use the Serial Monitor and tell me what is wrong !

Posted by: @zeferby

try to use this code to find a bug in your original version. This is your original code with the "hardware stuff" replaced with debugging code. Use the Serial Monitor and tell me what is wrong !

Eric you amaze me with your abilities to code and troubleshoot problems with code. I on the other hand have never coded in C and only dabbled a bit years ago with QBASIC & BASIC. That being said I am very proud of what I have been able to do with this robot project that I only have limited spare time to devote to it has made it more difficult because I need to have time to stay focused on my objective building a robot to navigate the house, which it does although the code is not very advanced and I have made ALOT of mistakes. Sometimes I get a bit confused with my own logic and I am amazed that anyone else can figure it out. 🙂 . All that being said I do want you to know I have sat here for hours trying to find my mistake in the aligntocompass code and I still have not been able to put my finger on what is wrong with it. Here is my serial monitor capture file ...

I can see there is a problem with 6 over through 6 under and I am not sure why it is doing that.

I know the issue is staring me in the face and I hate to admit but I don't understand why it is doing what it does.

Posted by: @zeferby

  Bug-hunting exercise 1 !

I think I failed exercise 1 Teacher but this is for you and its NOT bribery :0) .

So my understanding is that the robot uses the magnetometer readings to turn in a certain direction. It then moves in that direction a given distance using the encoders? Adjustments are made using data from the magnetometer to keep it in the right direction while moving until the given distance is reached? I tried to extract the direction and distance data from your code but I think maybe it should start and end at the same place?

My thoughts were to try and implement something similar to compare results and coding used. At the moment my robot relies only on encoder readings to travel in a straight line or turn about its center by a given amount of degrees.

Posted by: @casey

So my understanding is that the robot uses the magnetometer readings to turn in a certain direction. It then moves in that direction a given distance using the encoders? Adjustments are made using data from the magnetometer to keep it in the right direction while moving until the given distance is reached?

Yes your understanding is correct. At the present time I only use the right encoder to determine distance traveled, you may notice some small variations in my headings on this map and that is due to magnetic interference from things like my refrigerator, stove and in floor metal heating duct work.

Posted by: @casey

I tried to extract the direction and distance data from your code but I think maybe it should start and end at the same place?

One thing to keep in mind is my start heading is 108 and it gets this value from the Dheading variable in my global.