Welcome! Log In Create A New Profile

Advanced

Found bug in firmware 20100719

Posted by jmgiacalone 
Found bug in firmware 20100719
July 23, 2010 04:54AM
Hi all,

Found a bug in sStep(). As I'm running easydriver chips to run my steppers, I required ENABLE_ON to be high, and spotted that the extruder stepper enable code did not consider the constant INVERT_ENABLE_PINS as per the other motion axes. See change below:

void extruder::enableStep()
{
//digitalWrite(motor_en_pin, 0);//JMG 2010 07 22
digitalWrite(motor_en_pin, ENABLE_ON);
}

void extruder::disableStep()
{
//digitalWrite(motor_en_pin, 1);//JMG 2010 07 22
digitalWrite(motor_en_pin, !ENABLE_ON);
}


eMAKER-blue-2.png

[emaker.io]

[emaker.limited]
Re: Found bug in firmware 20100719
July 23, 2010 08:14AM
digitalWrite(motor_en_pin, !ENABLE_ON);

In case ENABLE_ON isn't set, this won't compile. In case it's set to 0, you'll switch the opposite, wrong setting. BTW., I can't find an ENABLE_ON #define in the FiveD firmware source code. Which firmware are you referring to?


Generation 7 Electronics Teacup Firmware RepRap DIY
     
Re: Found bug in firmware 20100719
July 23, 2010 08:27AM
Well, I downloaded the firmware from here a few days ago. File was reprap-mendel-20100719.zip

The configuration.h file has the following code:

// Set to 1 if enable pins are inverting
// For RepRap stepper boards version 1.x the enable pins are *not* inverting.
// For RepRap stepper boards version 2.x and above the enable pins are inverting.
#define INVERT_ENABLE_PINS 1

#if INVERT_ENABLE_PINS == 1 // *RO
#define ENABLE_ON LOW // *RO
#else // *RO
#define ENABLE_ON HIGH // *RO
#endif // *RO


So my code for enabling/disabling the extruder drive is copied from the following functions for the motion axes:
void cartesian_dda::enable_steppers()
{
#if MOTHERBOARD > 0
if(delta_steps.x)
digitalWrite(X_ENABLE_PIN, ENABLE_ON);
if(delta_steps.y)
digitalWrite(Y_ENABLE_PIN, ENABLE_ON);
if(delta_steps.z)
digitalWrite(Z_ENABLE_PIN, ENABLE_ON);
if(delta_steps.e)
ex[extruder_in_use]->enableStep();
#endif
}

void cartesian_dda::disable_steppers()
{
#if MOTHERBOARD > 0
//disable our steppers
#if DISABLE_X
digitalWrite(X_ENABLE_PIN, !ENABLE_ON);
#endif
#if DISABLE_Y
digitalWrite(Y_ENABLE_PIN, !ENABLE_ON);
#endif
#if DISABLE_Z
digitalWrite(Z_ENABLE_PIN, !ENABLE_ON);
#endif

ex[extruder_in_use]->disableStep();

#endif
}


I am of course assuming that !LOW = HIGH and !HIGH = LOW
Re: Found bug in firmware 20100719
July 23, 2010 02:24PM
#if INVERT_ENABLE_PINS == 1 // *RO
#define ENABLE_ON LOW // *RO
#else // *RO
#define ENABLE_ON HIGH // *RO
#endif // *RO

That makes sense.

Unfortunately, I'm not aware of any facility to report such bugs with patches, so posting a patch ("svn diff") here in the forum is probably the best you can do. Oh, and please remove the // *RO comments before doing so :-)


Generation 7 Electronics Teacup Firmware RepRap DIY
     
Sorry, only registered users may post in this forum.

Click here to login