variable not changed ... ?

Hi all,
I’m programming a ‘pen’ setup that involves 3 steppers, buttons and sensors. Individually I can make the different entities work on their own, but errors occur when they work ensamble. I’ve isolated one particular problem that conserns a float var ‘position’ that is in/decremented in the Step() function. It doesn’t seem to propagate all the way back to vars in the Loop().
The code is lengthy and messy and not representable.
The ‘pen’ class has a
fVector3 getPosition() & void setPosition(fVector3 ) where the position of the steppers are distributed.
From the pen-class:

	void cPen::setPosition(fVector3 posit ){
		// should be:
		cPen::mPosition = posit ;
		cPen::ewX.setPosition( posit.x ) ;//
		cPen::nsZ.setPosition( posit.z ) ;//
	}//cPen::getPosition-------------------------------------

	fVector3 cPen::getPosition(){
		// should be:
		cPen::mPosition.x = cPen::ewX.getPosition() ;//
		cPen::mPosition.z = cPen::nsZ.getPosition()  ;
		return cPen::mPosition ;
	}//cPen::getPosition-------------------------------------

ewX & nsZ are steppers
From the stepper-class:

	void cStepper::setPosition( float start_position ) //
	{ cStepper::position_x = start_position ; }  

	float cStepper::getPosition()
	{ return cStepper::position_x ; }

	void cStepper::Step()
	{		
  		if( cStepper::mDirection == true ){ cStepper::fase  +=1 ; cStepper::position_x += 1.0f ; }
  		else{ cStepper::fase  -=1 ; cStepper::position_x -= 1.0f ; }

  		if( 3 < cStepper::fase ){ cStepper::fase  = 0 ; }
  		if( cStepper::fase <0 ){ cStepper::fase  = 3 ; }

  		switch( cStepper::fase )
                .........

When testing position against rotary sensor-input, the getPosition() returns 0.
I’ve changed stepper float position to volatile in stepper.h, but not the fVector3.

I don’t know why it fails. The error occurs when the sensor is involved … it adds a lot of overhead to the code

[EDIT:] The sensor uses delayMicroseconds() … is that where the bugger is hidden?
I use the delay to get a proper analog reading, but is it really necessary?

where is position declared?

fkeel:
where is position declared?

cStepper-class
.h:
volatile float position_x ;// 1 pr step

.cpp [… constructor …]:
cStepper::cStepper()
{
cStepper::position_x = 0.0f ;

cPen-class
.h:
private:
fVector3 mPosition ;
.cpp:
cPen::cPen( cStepper &EastWest , cStepper &NorthSouth )
{
cPen::ewX = EastWest ;
cPen::nsZ = NorthSouth ;
cPen::xM = ewX.getMaxVelocity() ;
cPen::zM = nsZ.getMaxVelocity() ;
//mPosition = new cPoint() ;
cPen::mPosition ;
cPen::mPosition.x=0.0f ;
cPen::mPosition.y=0.0f ;
cPen::mPosition.z=0.0f ;
cPen::pi = 3.14159 ;
cPen::init() ;
}// end Constructor


The fVector3 is a struct that contains noting more than 3 floats.

[edit]
An additional error-symptom may be gleaned at the following:
After a motor-session I read old position from eeprom. If it’s changed, I’ll write the new position. /But/ … even if the two vectors (the eeprom-read and current from getPosition()) are serially written as (0,0,0), the equality fails none the less.

if( abs(cur.x - read.x)<0.01 && abs(cur.z - read.z)<0.01 ){
Serial.println("no save needed ") ;
}

You seem to have a fundamental misunderstanding of the purpose of classes, and are simply forcing a class to contain utility functions. None of this:

   cPen::ewX = EastWest ;
   cPen::nsZ = NorthSouth ;
   cPen::xM = ewX.getMaxVelocity() ;
   cPen::zM = nsZ.getMaxVelocity() ;
   //mPosition  = new cPoint() ;
   cPen::mPosition ;
   cPen::mPosition.x=0.0f ;
   cPen::mPosition.y=0.0f ;
   cPen::mPosition.z=0.0f ;
   cPen::pi = 3.14159 ;
   cPen::init() ;

should have cPen:: in front of it, if you are using cPen correctly.

I think you need to post ALL of your code.

By the way, files should have the same name as the class that they define. cStepper-class.h should be cStepper.h

Paul,
I don’t think that the post can contain all the code.
I’m all ears on the “You seem to have a fundamental misunderstanding of the purpose of classes”.

If I want a ‘container’ of data, I would make a structure.

The cPen class coordinates two steppers … they need two different but syncronized ‘velocities/delays’ to perform a line off the axis-directions. I successfully uses this (in cPen.cpp). Wouldn’t this be a utulity-function? … I’m confused and don’t really know what you mean.

	void cPen::moveLinear( float x , float z ){
		float flx = x*400 ;
		float flz = z*400  ;
		cPen::ewX.destination =  long( flx  ) ;
		cPen::nsZ.destination =  long( flz ) ;// so, what is x & z? .. still a round

Serial.print("ewX.destination  : "); 
Serial.println(cPen::ewX.destination); 
delay(50) ;

Serial.print("nsZ.destination  : "); 
Serial.println(cPen::nsZ.destination); 
delay(50) ;

		float timeSpent ;
		float xVel , zVel ; 
		if(abs(x)<abs(z)){
			zVel  =  cPen::mMaxVelocity ;
			timeSpent =abs( flz/cPen::mMaxVelocity );
			xVel = abs( flx/timeSpent)  ;
			}
		else{ 
			xVel  = cPen::mMaxVelocity ;
			timeSpent =abs( flx/cPen::mMaxVelocity) ;
			zVel = abs(flz/timeSpent)  ;
			}
		boolean forwX = false ;
		boolean forwZ = false ;
	    //both neg:
		if( x<0 && z<0 ){}
	    //both pos:
		else if( 0<=x && 0<=z ){
			forwX = true ; 
			forwZ = true ; 
		}
	   // mixed:
		else{
			if(z<x){ 
				forwX = true ;
			}
			else{ 
				forwZ = true ;
			}
		}
		cPen::ewX.setVelocity( xVel ) ;
		cPen::ewX.setDirection(forwX) ;
		cPen::nsZ.setVelocity( zVel ) ;
		cPen::nsZ.setDirection(forwZ) ;

	}// end cPen::moveLinear

The full cPen.h:

#ifndef cPen_h
#define cPen_h

#include "Arduino.h"
#include "cStepper.h"
#include "cButton.h"
#include "cPoints.h"
#include "cPoint.h"
#include "cArc.h"

#include "fVector3.h"

class cPen{
        public:
	cPen() ;
	cPen(cStepper & , cStepper & ) ;

	void ReSet() ;
void Enable() ;
void deEnable() ;
void Stop() ;
boolean setTime( boolean ) ;// 
	void goToLinear( cPoint ) ;
	void moveLinear( float , float ) ;
void goUnsyncronized(  float , float  );
//	void drawPoints( cPoints ) ;
//	void drawArc(cArc bue) ;
	fVector3 getPosition() ;
	void setPosition( fVector3 ) ;
	void setStepperEWX( cStepper & ) ;
	void setStepperNSZ( cStepper & ) ;
cStepper ewX ;
cStepper nsZ  ;

        private:	
	float mMaxVelocity ;
	fVector3 mPosition ;
	float pi ;
//	float sinFragX( int , float , float ) ;
//	cPoint divide( int , float , float ) ;
//	cPoints buildArc( cArc ) ;
	
	void init() ;
	float zM, xM ;//
	boolean boolNSZ ;
	boolean boolEWX ;
} ; //end class cPen

#endif

You spotted the cPen::pi and I intented to use it for controlling the syncronisation when drawing part of an arc. It’s lengthy and for now cut out of the class … but I don’t doubt that it will function too, if I set my mind to it. Wouldn’t
void cPen::moveLinear( float x , float z ){ … }
be what you call a utility-function?
As for “cPen::” I use it consequently in the .cpp and seem to have solved issues by doing so. But I’ll keep your suggestion in mind.

As for the problem about ‘position’ … I’ve posted the code where it’s altered.

So, let's talk about the code you posted:

    void cPen::moveLinear( float x , float z ){
        float flx = x*400 ;
        float flz = z*400  ;
        cPen::ewX.destination =  long( flx  ) ;
        cPen::nsZ.destination =  long( flz ) ;// so, what is x & z? .. still a round

This says that the cPen class has a moveLinear() method. To use this method, then, one needs to create an instance of the class, and call the method:

cPen inky;

inky.moveLinear(10.2, 45.7);

When this constructor is called, two cStepper instances are created. So, cPen::ewX and cPen::nsZ is not the way to access the methods. The cPen:: bit should not be there. That is needed only to access static methods or members of a class, and ewX and nsZ are not static members.

When this constructor is called, two cStepper instances are created. So, cPen::ewX and cPen::nsZ is not the way to access the methods. The cPen:: bit should not be there. That is needed only to access static methods or members of a class, and ewX and nsZ are not static members.

I think this is a misunderstanding. I do create an instance of the cPen class (called pen)
Program code:

#include ----


const int x_EA = 41 ;
const int x_i2 = 39 ;
const int x_i1 = 37 ;  
const int x_EB = 40 ;
const int x_i4 = 38 ; 
const int x_i3 = 36 ;

cStepper stpX( x_EA, x_EB, x_i1, x_i2, x_i3, x_i4 ) ;
...
cStepper stpY( y_EA, y_EB, y_i1, y_i2, y_i3, y_i4 ) ;
...
cStepper stpZ( z_EA, z_EB, z_i1, z_i2, z_i3, z_i4 ) ;
...

const int relayPin = 43 ;// 51 ;// 43 relay hooked on yPin

//sensors:
const int wheelXpin = A0 ;
const int wheelZpin = A1 ;// should be active
const int joyXpin = A6 ;
const int joyZpin = A7 ;

cPen pen( stpX , stpZ ) ;

[notice the last line]

I’m trying to be consequent about using pen.ewX instead of referring directly to stpX, but as you’ll see in the following code, I’m not.

More code
All this functions well … no problems until I involve an analog sensing (executed in every setTime() [=check to see if stepping should occur] )

in cStepper.cpp:

	boolean cStepper::setTime( boolean HalfOrFull ){
		if( HalfOrFull ){ cStepper::isHalfStep = false ; } 
		else{ cStepper::isHalfStep = true ; }

		//if( cStepper::oldTime == 0 ){  cStepper::oldTime = micros() ; } //microsSinceLast = cStepper::mDelay ;
		boolean didExeed = false ;
		boolean res7 = true ;
			
		if ( cStepper::misOn ){ 
			long microsSinceLast = micros() - cStepper::oldTime  ;
			if(microsSinceLast >= 2*cStepper::mDelay){ microsSinceLast = cStepper::mDelay ;}

			if(microsSinceLast>=cStepper::mDelay ){cStepper::MicrosDiscrepancy = microsSinceLast - cStepper::mDelay ;}
			if( (cStepper::oldTime + cStepper::burnIn) <  micros() ){
				// if delay more than burnIn then holding is abandonned
				cStepper::Pause() ;
			}// end if oldTime + burnIn 
			
			
			if(  ( (cStepper::oldTime + cStepper::mDelay) < micros() - cStepper::MicrosDiscrepancy ) ){
				// mDelay microseconds has passed
				if( isGoingFree  ){
				// no consideration on destination
					if( HalfOrFull ){ cStepper::Step() ;} 
					else{ cStepper::StepHalf() ;}	
					res7 = true ;
					cStepper::oldTime = micros() ; 
				}
				else{
					if( cStepper::HalfStepCounter  < abs(cStepper::destination ) ){
					// if stepper has not reached destination:
                                       		 cStepper::oldTime = micros() ; //- ad ;
						if( HalfOrFull ){ cStepper::Step() ;} 
						else{ cStepper::StepHalf() ;}
					
						res7 = true ;
					}
					if(cStepper::HalfStepCounter  == abs( cStepper::destination) ){
						cStepper::destination = 0 ;
						//cStepper::stepCounter 
						//cStepper::misOn = false ;
						//cStepper::misOn = false ;
						cStepper::Pause() ;
						//position_x
						Serial.print(cStepper::name);
						Serial.print(" at destination ");
						Serial.println(cStepper::position_x);
						Serial.print("HalfStepCounter : ");
						Serial.println(cStepper::HalfStepCounter);
						delay(20) ;
						res7 = false ;
					}
					if(HalfStepCounter  > abs( cStepper::destination) ){
						//String nm = cStepper::name ;
						//Serial.print( nm );
						Serial.println(" exeded" );
						delay(5) ;
						didExeed = true ;
						//cStepper::deEnable() ;
						//cStepper::misOn = false ;
						cStepper::Stop() ;
						res7 = false ;
					}
				}// end isGoingFree
			}// end if (oldTime + mDelay) < micros
		
		}// end if cStepper::misOn
		else{
			res7 = false ;
			//Serial.println("setTime are off");
			//delay(10) ;
		}

		return res7 ;		
	} // end cStepper::setTime //-----------------

This function needs to be ‘listened’ into to drive the steppers (it’s the only code that calls Step() )

the pen uses the function:
from cPen.cpp

	boolean cPen::setTime( boolean HalfOrDouble ){
	// a true' return means that steppers are not 'home' and need to continue
		boolean res6, zt, xt ;
		// when destination == 0  no needto go to setTime
		if( cPen::nsZ.destination == 0 ){  zt = false ; }//destination is long
		else{ zt = cPen::nsZ.setTime( HalfOrDouble ) ; 

		}
		if( cPen::ewX.destination == 0 ){ xt = false ; }
		else{ xt = cPen::ewX.setTime(HalfOrDouble) ;  }
		if (zt == false &&  xt == false){
			res6 = false ;
		}
		else{ res6 = true ; }	
		return res6 ;	
	} // end boolean cPen::setTime ----------------------------

I call the pen function in the program. First by

void xzRepeatCircular( float Radius , int count , boolean DrillOnDown , float drillDepth ){
  float distX1 , distY1 , distX2 , distY2 , rad , cPi, absX, absY ;
  cPi = 3.14159f ;
  rad = (360/count)*(cPi/180 ) ;
  // const float convert = 180/pen.pi ; 
  distX2 = 0.0f ;
  distY2 = 0.0f ; 
  for( int i = 0 ; i<count ; i++ ){
    distX1 = Radius*cos(i*rad) ;
    distY1 = Radius*sin(i*rad) ;
    absX += distX2 - distX1 ;
    absY += distY2 - distY1 ;
    LoopPen(distX2 - distX1 , distY2 - distY1 , false ) ;// false for using halfStepp
    distX2 = distX1 ;
    distY2 = distY1 ;
    // do drill
    if(DrillOnDown){
      MoveY( -drillDepth , true ) ;
      MoveY( drillDepth , false ) ;
      //should listen in on button-events
    } // drill?
  } // end for

This function utilizes program-function LoopPen()

//moves distance x,z                  fullStep for true
void LoopPen(float mX, float mZ , boolean HalfOrFull ){
  boolean res = true ;
  boolean xbool = true ;
  boolean zbool = true;

  pen.nsZ.Shift(true);
  pen.ewX.Shift(true);
  LoopCounter=0;//variable in sensors

  //Serial.println("enter LoopPen");
  //delay(100) ;
  float x,z ;
  if( HalfOrFull ){ 
    x = mX ; 
    z = mZ ;
  }// fullStep
  else{
    x = 2*mX ; 
    z = 2*mZ ;
  }// halfstep
  pen.moveLinear( x , z ) ; // sets dir and speed .. im mm
  //Pen moves:
  while ( res ){ 
    if( HalfOrFull ){  
      if(z!=0 && zbool){   
        zbool = pen.nsZ.setTime(true) ;
      }// true for fullStep
      else{
        zbool=false;
      }   
      if(x!=0 && xbool){   
        xbool = pen.ewX.setTime(true) ;  
      }
      else{
        xbool=false;
      }   
      //LoopSensor() ;
      //      outLoopSensorZ = LoopSensor();
      if( (zbool==false) && (xbool== false) ){
        res = false ;
      }
    }
    else{
      if(z!=0 && zbool){  
        zbool = pen.nsZ.setTime(false) ;
      }// true for fullStep
      else{
        zbool=false;
      } 
      if(x!=0 && xbool){  
        xbool = pen.ewX.setTime(false) ;
      }
      else{
        xbool=false;
      }  
      LoopSensor() ;
      if( (zbool==false) && (xbool== false) ){
        res = false ;
      }
    }
    //    outLoopSensorZ = LoopSensor();
  } // looping while
  Serial.print("sens ") ;
  Serial.println(LoopCounter) ;
  stpZ.Shift(false);
  stpX.Shift(false);
  stpZ.Pause();
  stpX.Pause();
} // end LoopPen--------------------------------------------------------------------

Your code is nearly impossible to read.

        //if( cStepper::oldTime == 0 ){  cStepper::oldTime = micros() ; } //microsSinceLast = cStepper::mDelay ;

There really is no excuse for putting multiple statements on one line. Spread them down the page.

There are debates about whether the { goes on the line with the statement or on a new line. There is no debate anywhere about where the } goes. It goes ON ITS OWN LINE.

        if( HalfOrFull ){ cStepper::isHalfStep = false ; } 
        else{ cStepper::isHalfStep = true ; }

Is isHalfStep declared static in the cStepper class? If not, then this code belongs to an instance of the cStepper class, which has its own isHalfStep member. So, the scope resolution operator should not be used.

        if ( cStepper::misOn ){

Same question for the misOn member. Is it static? If so, why?

                cStepper::Pause() ;

Is the Pause() method static? If so, why? If not, loose the cStepper:: in front of it.

I'm trying to be consequent about using pen.ewX instead of referring directly to stpX, but as you'll see in the following code, I'm not.

Consequent? No idea what that means. If it was supposed to be consistent, then I don't see a relationship between the statement and the code.

Names like res7 don't mean anything. Can't you think of a reasonable name?

This function needs to be 'listened' into to drive the steppers (it's the only code that calls Step() )

No idea what this means. Functions get called, not listened to.

All this functions well .. no problems until I involve an analog sensing (executed in every setTime() [=check to see if stepping should occur] )

I don't see any calls to analogRead() in the setTime() method. How does a name containing a noun (Time) and a verb (set) like this one relate to "a true' return means that steppers are not 'home' and need to continue". Meaningful names, again.

Paul,
Yes, I deserve a spanking on my way of presenting code. There are a couple of misnomers. Polling is when a pin is read for it’s state … I’ve used ‘listeningto’ for functions that is polled or executed often (as setTime() is ).
res7 is a boolean result that is returned. At some time I worried that all the standard boolean results I would return from the class (res) would be intermixed.

As for prepending all members of say the cPen class with cPen:: … I thought that it was mandatory. It makes sense that only static variables of the class gets the honor. It’ll take time to revise the code for that error.

The analog sensing is not executed in setTime(), but parallel to it, sorry. I call it with LoopSensor() in function LoopPen() (only half of the LoopSensor() ocurrences are commented out, error).

The LoopSensor() leaves a lot to be desired, but I’m not going to change it until I know what to change it to. Currently I’m investigating different radical options, but all of them suffers from the fact, that the sensed signal is not very clean.

The following code catches 35-40 ‘ticks’ out of 48. The
if(mmytime + 150> micros() ){ … }
is a kind of bounce-prevention that should discard quick spikes on the signal. If it’s not there, the sensor will catch +100 ‘ticks’

A dial on the stepper-shank has 24 holes giving 48 changes/ticks pr round. It’s executed 200 times pr round (in sync with setTime() ). I’ve been on issues conserning this in other threads, but without getting anywhere.
Luckily the steppers works better now due to ie keeping track of “time-discrepency/overflow” in the setTime(), and I may do without checking the achtual steps that’s performed.

int sensorValue = 0;  // 
int sensorValueOld = 0;
int sensorVeryOld=0;
int veryvery =0;
int very4 =0;
int very5=0;

int maxVal; //= 900 ;
int minVal; //= 5 ;

int ct=0;
long mmytime, oTime;
boolean isHigh, isHighOld ;


int LoopSensor(){
  sensorValue=0 ;
  for(int i = 0 ; i < 3 ; i++){
    sensorValue += analogRead(wheelZpin);
    delayMicroseconds(130);
  }
  sensorValue /=3 ;

  maxVal= 600 ;
  minVal= 15 ;

  if(sensorValue < minVal && sensorValueOld<minVal && sensorVeryOld<minVal && veryvery<minVal && very4<minVal&& very5<minVal ){
    isHigh = false ;
    //digitalWrite(ledPin, LOW );
//    countMe();
  }
  else{
    if(sensorValue>maxVal && sensorValueOld>maxVal && sensorVeryOld>maxVal && veryvery>maxVal && very4>maxVal && very5>maxVal){
      isHigh = true ;
      // digitalWrite(ledPin, HIGH);
//      countMe();
    }
  }

  very5 =very4 ;
  very4 = veryvery ;
  veryvery=sensorVeryOld;
  sensorVeryOld=sensorValueOld;
  sensorValueOld=sensorValue;

  if(isHigh !=isHighOld ){
    isHighOld=isHigh;    
    mmytime=micros();
  }  

  if(mmytime + 150> micros() ){
    countMe();          
  } // 
  return LoopCounter ;
} //end int LoopSensor()---------------------------------

boolean tick;
int countMe(){
  if(tick){
    digitalWrite(testPin, HIGH); 
    tick=false;
  }
  else{
    digitalWrite(testPin, LOW); 
    tick=true;   
//  Serial.println(LoopCounter) ;  
  }    
  LoopCounter++;
  return LoopCounter;
}

I read somewhere, that delay() and Serial. uses interrupts, and that they in turn does not keep track of a status-register … whatever this means, it got me thinking that this could be how the ‘position’ got lost. The above sensor-code has nothing to do with the ‘position’ variables.

Paul, I appreciate the time & effort you put into this.

… if the program ‘think’ that stepper.position is a static var … then changes in stepperZ will flow into stepperX, right? I think this could be the culprit.

Carsten

for reference: code from cStepper.cpp

//------------------------------		
		void cStepper::Enable(){
  			analogWrite( cStepper::x_EA , 255 ) ;
			delay(20) ;
  			analogWrite( cStepper::x_EB , 255 ) ;
			delay(20) ;
			cStepper::misOn  = true ;
			cStepper::HalfStepCounter  = 0 ;
			//cStepper::position_x = 0 ;
			cStepper::oldTime =  micros() ;
	Serial.print(cStepper::name);
     Serial.println("-Enabled"); 
     delay(20) ;
		}

		void cStepper::deEnable(){
  			analogWrite( cStepper::x_EA , 0 ) ;
  			analogWrite( cStepper::x_EB , 0 ) ;
			cStepper::setVelocity( 0.0f ) ; 
			cStepper::misOn  = false ;
			cStepper::destination = 0 ;
			cStepper::Pause() ;
			delay(20) ;
	Serial.print(cStepper::name);
     Serial.println("-deEnabled"); 
     delay(20) ;
		}

		void cStepper::Pause(){
  			digitalWrite( cStepper::x_i1 , LOW ) ;
  			digitalWrite( cStepper::x_i2 , LOW ) ;
  			digitalWrite( cStepper::x_i3 , LOW ) ;
  			digitalWrite( cStepper::x_i4 , LOW ) ;
			//cStepper::stepCounter = 0 ;
	//Serial.print(cStepper::name);
     //Serial.println(" Paused"); 
     //delay(20) ;
		}

		void cStepper::Stop(){
  			digitalWrite( cStepper::x_i1 , LOW ) ;
  			digitalWrite( cStepper::x_i2 , LOW ) ;
  			digitalWrite( cStepper::x_i3 , LOW ) ;
  			digitalWrite( cStepper::x_i4 , LOW ) ;
			cStepper::setVelocity( 0.0f ) ; // = 0.0f ; 
			cStepper::HalfStepCounter  = 0 ;
	Serial.print(cStepper::name);
     Serial.println( " Stopped"); 
     delay(20) ;
		} // void cStepper::Stop() --------------------

Paul, I took a check in Deitel & Deitel's 'How to program C++' about the binary scoop resolution operator :: According to that book I use it the way it's supposed to be used (if it was C++) .. shall I understand you as: The arduino programmng language is different? My classes are all stored in .h and .cpp files in the library.

Scope resolution is only needed if there's some ambiguity - from a brief scan of your code, most times you've used it, there is no ambiguity. Arduino [u]is[/u] C++.

AWOL: Scope resolution is only needed if there's some ambiguity - from a brief scan of your code, most times you've used it, there is no ambiguity. Arduino [u]is[/u] C++.

Good .. that leaves the usage out of the question as an error. Not to appear ungreatfull, but Paul has been all over my code and even critizised an out-commented line for being mal-formed .. but I don't recall that he anywhere as addressed my issue. If you are fresh on it, then I can recap, that 1) using getPosition() returns a (0,0,0) vector where the vector is not (0,0,0) 2) the (0,0,0) is how the result appears in Serial.print(position-vector). When that vector is compared to another (0,0,0) vector, there is no equality.

[EDIT] I'm setting the position vector in 3 places (and I've been meticuolous about checking) in penEnable() (used once at start) in reading from eeprom (no errors detected) and setting it's value at start in Step() where each component (x,z) is incremented ... and it works until I start executing the sensorLoop()

I'm currently investigating if I'm over-extending my board

You are asking questions that require us to look at ALL of your code. You have posted snippets here and there.

Most likely, when you get unexpected outputs like you are seeing, the scope of a variable is wrong. We see a lot of people creating a new variable, as a local variable, with the same name as one with larger scope. Without seeing all of your code, though, that is only as guess as to what your problem might be.

There is another possibility, and that is that you appear to expect some function to return a vector. Functions cam only return single values. Of course, that value can be a compound object (pointer, struct, instance of a class, etc.). If the object IS a compound object, and that object was created on the heap, that object will go out of scope when the function ends. If you have returned a pointer to that object, rather than the object itself, then the pointer will be garbage. Referencing it anyway will give unpredictable results.

Another common issue is to expect integer arithmetic to produce float results.

So, you see, there are any number of things that could be wrong with your code. That is why we need to see it all.

Hi Paul, I've gathered the active code in a .txt at http://troelsgaard.net/images/ct_Images/Positioner/Arduino.txt

It's largely uncommented and I'll use this thread for that. It's an unforgivable task to look for errors in a pile of junk like that, but I will assume that you may be able to use a proper editor for error-detection or search/find. I have written the class-code in notepad and program-code in the Ardino-editor. The code is segmented into the class-code and meaningful taps in the Arduino editor (like Setup() & Loop(), Sensors, (Button) commands, Tasks ..)

I've build a line-in button-commander that uses 3 buttons to access and execute code. Conceptually the commands that are executed are 'kept' in what I think of as a table-of-commands. Two buttons controls row and column (selectors), the third executes the selected command. It's using some lengthy switch case: one for popping me a message on which table-entrance I just went into, another for holding the function to execute. The table-functions are (for now) largely for reading and writing to eeprom and SDCard, but it has 'goTo's too. Two other buttons on the button-commander controls on/off and a pause.

Monitor-input converts input to action (moving steppers). It's been build ad-hoc and is not pretty. I think of it as an alternative to move the pen by the joystick.

Implementing the joystick meant changing some code in the central stepper.setTime() .. to make it go directly to Step(). I try to make steppers go as fast as possible: they are inherently slow and they make less noise on high speed. But it clutters the code at places.

Skip any error-finding on what concerns blinking leds.

Please ask in on more detailed commenting.

I don't feel skilled enough to use pointers, references and addressOf where it would be advantageous.

I’ll be making comments as I read your code.

cButton::cButton( const int buttonPin )
{
	cButton::mButtonPin = buttonPin ;
	cButton::mEnabled = false ;
	cButton::debounceDelay = 50 ;
	pinMode(mButtonPin, INPUT) ; 
	cButton::hasLed = false ;
	cButton::mAsAnalog = false ;
	cButton::isActive = true ;
	cButton::mTheSecond = true ;
}//end constructor

You have no control over when the constructor will be called. The user might create an instance inside a function, in which case this constructor will work properly. On the other hand, the user might create a global instance, in which case the constructor will be called before the init() function is called.

Since you do not know when the constructor is called, you should not do things that rely on init() having been called, like set the mode of a pin.

Your class should have a begin method that must be called in setup() or before the instance is used (as in Serial.begin()) in which the pinMode() function is called.

The random indenting is hard on the eyes and on the brain. I’d recommend that you ditch notepad as a code editor. Notepad++ is orders of magnitude better.

	}// if( isActive ){

No coding style anywhere recommends anything after the }, except a comment. That does NOT mean that it is OK to put commented out code there.

You cButton class is full of cButton:: stuff, still. Ditto the cStepper class.

The debounce logic in your ReadPin() method is completely backwards. I can’t follow it.

There is already a Button class that provides nearly all the capabilities that your does. Deriving from that class to add the extra stuff would have been a better idea.

	boolean cStepper::areOn(){ 
		if( cStepper::misOn == true ){ return true ; }
		else{ return false; }

Why not something much simpler?

	boolean cStepper::areOn() { return misOn; }

Every character you type is a potential place for a bug to creep in. Provide fewer such place by typing less.

		if( cStepper::misOn  == false ){}// cStepper::Stop() ; cStepper::mVelocity = 0.0f ;

Why is this here at all? If misOn is false, do nothing. Otherwise, do nothing.

Multiple statements on one line are never advised. Modifying such code is ridiculously difficult.

	volatile float position_x ;// 1 pr step

Why is this variable volatile?

			if(  0 < cStepper::mVelocity ){

If 0 is less than mVelocity… Is that really the way you talk? The operators in C were designed to allow you to construct comparisons in the same way that you think. Turn this around properly.

But, this whole function needs rework. You accept the input velocity value, even if it is crap. Then, you return a value that says whether the input was crap, or not. Only when the input is not crap should the member field be set to the input value.

				float r = 1000000/cStepper::mVelocity ;

Literal constants are interpreted as ints, in the absence of directives to the contrary. There are no such directives, and 1000000 is most certainly not an int. What will this code do?

You should make sure it does what you want by fixing the code. Either declare that 1000000 is a long, by appending an L or UL, or add a .0 after it to make it a float.

			if(velocity ==0){cStepper::mDelay = 1000000 ;}

Two problems here. The first is the same constant that could be misinterpreted. The second is that velocity is a float, and will probably never be exactly 0.00000000000. Equality involving a float is rarely a good idea.

		float t = 1000000/dly ;
		 cStepper::mMaxVelocity =  float(t ) ;

Why are you casting a float to a float?

		boolean didExeed = false ;// not used

Then delete it.

	boolean cStepper::setTime( boolean HalfOrFull ){

This method name is not even vaguely related to what this function is doing.

  		if( 3 < cStepper::fase ){ cStepper::fase  = 0 ; }
  		if( cStepper::fase <0 ){ cStepper::fase  = 3 ; }

Ass backwards logic on one statement, but not on the other. Consistency is a GOOD thing.

There are precious few Serial.print() statements in the code. As far as your original issue, nothing jumps out at me as being obviously wrong. I’d add debug statements to validate that all of your assumptions are true.

ListenToSerial() has some issues. After each character read, you call ParseParameters() or ParseString(). You should only call those functions when the end of a packet has been detected.

The use of the String class is generally to be avoided, except in small programs. Yours is anything but a small program. The things that you can do with a String object can be done with a string, too. If you don’t know how, learn how.

Paul,
Thank you for looking and giving critique,

I have two habits: One is consistently to use only < to prevent me from any confusion when reading those signs. Recently I realized that the value being compared ‘always’ is on the left side. I see that I haven’t got to correct all.
The other habit is to mark the ending bracket } with at commented hint of where to find the start. I find it useful and supportive, and I’ll continue with it; but now you know why and what the comment relates to.

I’m aware of the potential issues that can arise around the init() function. I use it to serve one particular problem: to have the cPen.MaxVelocity() value sat (from the minimum value of one of the two stepper.maxVelocities() if memory serves me right). It’s observant of you to catch sight of it.

I have an unreasonable lot of errors from poor conversions between types. Not to get hung up on it in the middle of something else, I’ve sometimes chosen to insert extra variables. Poor coding.

When I wrote the cButton-class (it was the very first in these programming environments), I hoped that I could delegate all the nitty gritty to it. I should have realized the consequence of polling and interrupts (soo used to the microsoft event-model). One can safely say that that class failed to be of any help.

Before sending the code to you, I erased some of the commented-out lines relating to Serial feed-back. At some time the Serial statements would spawn an error (maybe I was using it in a constructor or before setting Serial.begin() ). Anyway, where I would like to use it (in ie. setTime() ) it is way too slow to keep up with events or it ruins proper execution.

As for the string and String … … the notion of pointers gives me a rash. I have messed around with my first trial & error-code in the read/write to eeprom, so I have probably passed the first difficult barrier to get to learn it. I’ve begun to think of using it for the position-variable that is the root-problem of this thread.

In spite of stern efforts the setTime() has failed to live up to a must convey intuitive info about what it does/is. I recently met ‘check’ that could be used instead, but only time (or you) can tell if it’s right. The Time() functions stresses me because they are often used where efficient code is of the essence (called often). I chose the name because I would intuitively recognize it as a function that would be called often.
There are two kinds of code in my world: the must be extremely efficient … and the rest. I love the rest because I can focus on writing something understandable, something that will be recognizable some time in the future too.

You can probably guess, that I all in all has spent a lot of time on programming. This has been a rare opportunity to talk about what I do. Thank you.

Carsten

The other habit is to mark the ending bracket } with at commented hint of where to find the start. I find it useful and supportive, and I'll continue with it; but now you know why and what the comment relates to.

That's fine. I find it easier to place each { on a new line, and line the } up with it, so I can see the start { that corresponds to an end }.

I tend to keep functions short, so that when printed, the whole function is on one page.

Using comments to define what the } matches is fine. Putting other code on the same line was what I was objecting to.

I have an unreasonable lot of errors from poor conversions between types. Not to get hung up on it in the middle of something else, I've sometimes chosen to insert extra variables. Poor coding.

I disagree. Extra variables can often make code easier to follow. Returning true is some variable contains true and false if the variable contains false is not one of those places, though. Simply return the variable's content directly.

Before sending the code to you, I erased some of the commented-out lines relating to Serial feed-back. At some time the Serial statements would spawn an error (maybe I was using it in a constructor or before setting Serial.begin() ). Anyway, where I would like to use it (in ie. setTime() ) it is way too slow to keep up with events or it ruins proper execution.

This is a legitimate problem. But, it is a problem that forces you to get creative. In the case of actually driving an NC machine, speed is important. Calling the function from a test driver, speed is not at all important. I suspect that you need to develop a sketch that has a number of test driver functions, to validate that setTime(), and other functions, are completely bug free independent of other functions.

When all functions are known to be bug-free, then testing a program is a simple matter of making sure that the bug-free functions are called in the correct order.

You can probably guess, that I all in all has spent a lot of time on programming. This has been a rare opportunity to talk about what I do.

The size of that program alone indicates that it is not your first program. It appears that C++ is not your favorite language to program in. Removing the extraneous stuff from your code (the class name :: stuff) would go a long way towards making your code easier to understand. While using the scope resolution operator is not technically wrong, it is rare enough to cause people to wonder why it is done.

Good luck on your project.

Good luck on your project.

Thanks a lot .. I've got a 3d-scanner on the bedding, so I'm sort of eager to round the 'positioner' off.

Last night I got to think, that poll() could be a right name for stepper.setTime() (and button.Read() ), but it's still slightly off: in an event-perspective I actually sets all boolean variables for all the sub-events and states (of the button). The snag still is, that each boolean value of a sub-event/state has to be polled. pollStep() could leave all doubt behind.

Broadcasting says that summer is comming up in EU (about time ;o/) - it seemed to be stuck on your continent ;o)

it seemed to be stuck on your continent ;o)

Not my corner of it!

I would like to add to Paul’s detailed critique. Using the scope resolution operator (::slight_smile: where not required is very confusing. Please don’t do it.

Inside a class method, you normally would never do that, since the class variables belong to the class, the scope operator is only used to resolve an ambiguity (eg. base class compared to derived class).

You already appear to be using the “m” prefix to make it clear that this is a class variable. That should be enough.

I also hate this style, whatever reason it might have:

if( 3 < cStepper::fase )

That’s like saying:

If 3 pm is less than the time …

rather than

If the time is 3 pm or later …

No-one talks like that (the first example).