Not all of function code is executing!

I have built the insect robot described in “Arduino Bots and Gadgets” and having got the base movement code working I decided to rewrite it in a more modular form, but it seems that only the first four lines of the code of my “Move::forward()” function are actually executing based on the serial debugging output. In addition the values passed the the function are not correct. Why would this be?

main.cpp:

#include "WProgram.h"
#include "LED.h"
#include "Move.h"

// Walking speed, between 1 (slow) and 5 (fast).
int walkSpeed = 3;

// Mechanical centre trim positions of the servo's.
int frontServoCenter = 97;
int backServoCenter = 100;

LED *led;
Move *insect;

void blink() {
	led->blink(1, 1);
}

int main() {
	init();
	Serial.begin(115200);

	*led = LED(13);
	*insect = Move(2, 3, frontServoCenter, backServoCenter);

	while (true) {
		insect->forward(walkSpeed);
		blink();
	}

	//Unreachable code but it's required by the compiler
	return 0;
}

Move.cpp:

#include "Move.h"

const char Move::format[] = "%s: %d,%d";

Move::Move(int frontServoPin, int backServoPin, int frontServoCenter, int backServoCenter) {
	frontServo.attach(frontServoPin);
	backServo.attach(backServoPin);

	frontCenterPos = frontServoCenter;
	backCenterPos = backServoCenter;
	frontRightUp = frontCenterPos - 18;
	frontLeftUp = frontCenterPos + 18;
	backRightForward = backCenterPos - 15;
	backLeftForward = backCenterPos + 15;
}

void Move::forward(int walkSpeed) {
	position("forward-1", frontRightUp, backLeftForward);
	motionDelay(125);
	center("forward-2c");
	motionDelay(65);
	position("forward-3", frontLeftUp, backRightForward);
	motionDelay(125);
	center("forward-4c");
	motionDelay(65);
	speed(walkSpeed);
}

void Move::position(const char* action, int front, int back) {
	log(action, front, back);
	frontServo.write(front);
	backServo.write(back);
}

void Move::center(const char* action) {
	position(action, frontCenterPos, backCenterPos);
}

void Move::speed(int walkSpeed) {
	if (walkSpeed < 1) walkSpeed = 1;
	if (walkSpeed > 5) walkSpeed = 5;

	int millisDelay = 300 - (50 * walkSpeed);
	motionDelay(millisDelay);
}

void Move::motionDelay(int millis) {
	Serial.println(millis);
	delay(millis);
}

void Move::log(const char* message, int x, int y) {
	char buf[256];
	snprintf(buf, sizeof(buf), format, message, x, y);
	Serial.println(buf);
}

Serial debugging output:

forward-1: 79,0
125
forward-2c: 97,100
65
forward-1: 79,0
125
forward-2c: 97,100
65
forward-1: 79,0
125
forward-2c: 97,100
65
forward-1: 79,0
125
forward-2c: 97,100
65
forward-1: 79,0
125
forward-2c: 97,100
65
forward-1: 79,0
125
forward-2c: 97,100
65
forward-1: 79,0
125
forward-2c: 97,100
65
forward-1: 79,0
125
forward-2c: 97,100
65

Hmmm, code looks straightforward - have you tried uploading again? Does it report any errors on uploading?

I have upload loads of times and tinkered with everything I can think off, it almost seems like the code is getting corrupted, is my pointer stuff ok, my C++ is very rusty.

256 bytes is quite a big buffer on a machine with 2k RAM in total. Could you try reducing it to, say, 64 bytes and see if anything changes?

Remember that all your string constants (in double quotes) will also reside in RAM, even if they’re never modified.

My c++ is rusty too, but I can't see that you have allocated any memory for led or insect. Both of them appear to be pointers to address 0. They'll step on each other, as well as whatever the compiler put there. The large buf variable might be a problem, but I can't see why the third use of it is any more likely to cause an issue. Use of millis as a variable name is valid, but since there is a standard function of the same name, not advisable.

Ah, well spotted! I didn't see those uninitialised pointers!

So how do initialize them?

Move Insect(frontPin,backPin,frontServoCenter,backServoCenter);

The Arduino doesn't implement new or delete by default. There is no particular advantage to using pointers, especially not for LED.

Change:

LED *led;
Move *insect;

to:

LED led;
Move insect;

The Move constructor will be called to create insect.

There are other problems that need to be addressed.

The init() function initializes the hardware. So, the LED and Move constructors can not reference hardware related things like pins. Each class should have an init() method that gets called in main, before using the objects.

Thanks for your help, I have created a begin() method which attaches the servos, instead of doing this in the constructor, then called begin() after init():

#include "WProgram.h"
#include "LED.h"
#include "Move.h"

// Walking speed, between 1 (slow) and 5 (fast).
int walkSpeed = 3;

// Mechanical centre trim positions of the servo's.
int frontServoCenter = 97;
int backServoCenter = 100;

LED led = LED(13);
Move insect = Move(2, 3, frontServoCenter, backServoCenter);

void blink(LED led) {
    led.blink(10, 1);
}

int main() {
    init();
    Serial.begin(115200);
    insect.begin();

    insect.standStill(5);
    while (true) {
        insect.forward(walkSpeed);
        blink(led);
    }

    //Unreachable code but it's required by the compiler
    return 0;
}

There is also a good discussion of why you need to do this here: http://www.arduino.cc/cgi-bin/yabb2/YaBB.pl?num=1294610218