Bugs found in G-Code parser

I'm using the arduino code in my project, and in going through it I've found number of bugs. I'm not sure who's maintaining the code so I thought I would just list the issues here. I don't want to sound rude, but it amazes me that this code ever worked.

-----------------------
if a command has X,Y, or Z in it but not G, you look for the Gcode in it.

if (has_command('G', instruction, size) || has_command('X', instruction, size) || has_command('Y', instruction, size) || has_command('Z', instruction, size)) {
code = (int)search_string('G', instruction, size);

for the search_string to return a valid value, you must have a G in the command, so why check for the X,Y,Z? If you have an X/y/Z but no G then the search_string command will return garbage that you would act upon
------------------------------
you have two case statements that act on the output of the Gxx code - why? just combine them.
------------------------------
You only extract the X/Y/Z locations if you have a G3 code in the first block, then try to use those values in the second block, which will not work.
------------------------------
//do we have a set speed?
if (has_command('G', instruction, size)) {

should be

//do we have a set speed?
if (has_command('F', instruction, size)) {
-------------------------------------


you know, nevermind, it's going to take too long to document all the problems, I'll just make the fixes and post the updated code. Or am I looking at the wrong version? I'm looking at the non-experimental version.

Hi

Exactly which version of the firmware are you looking at? I keep track by the name of the zip file they are in. If not in zip please give download location.

---

Regards

Ian

Ok, I've done a bunch of major rewriting, except to the G03 code as I'm not certain how that part is supposed to work.

I've also condensed the code a lot and put a lot of redundant actions into functions (makes it more legible for me - personal preference), added a string parsing routine for the gcode commands which generates a linked list of tokens that simplify and speed up extracting values. I couldn't follow the complex logic of figuring out which axis should step so I rewrote how that's all handled. A bunch of other small changes such as keeping track of the dda_move() function's overhead and subtracting it out from the inter-step delay.

I've just finished with the writing part, need to debug and make sure it works now. On the negative side, I did remove support for the extruder and the M codes (easy to add back though under the new paradigm). I've also removed the code for the limiting switches as my cnc doesn't have them.

I'll post after testing & debugging.

Hi

That will be interesting. Could you include code for limits with a define to switch them on or off as most other people use limits.

They will also make setting MUCH easier if you use a homing routine.

---

Regards

Ian

It would be trivial to add limit switch support, though i'll leave it commented out so it doesn't interfere with my keypad interface. I'm also curious what you mean by a homing routine.

I'm mostly happy with it, though there are a couple issues that perhaps someone with some experience can help me with.

1. The original code stepped one channel at a time. I believe this could lead to jagged edges and feel that the various motions should be synchronized. I would ideally like to write the step(s) simultaneously, which you normally can do on an AVR using the command

PORTB = _BV(1) | _BV(2) | _BV(3); //set three bits high

however, in the arduino environment, this doesn't work. The best I can get, with three consecutive DigitalWrite statements is a delay of 7uS between the start of one stepper to the start of the last one. There must be a way to set a port on the ardino, not just a pin. Any ideas?

2. I rewrote the code that figures out how to pulse the steppers to get linear 3d motion at a constant speed, but based it on the original code: figure out the max speed for the axis that's going to move the furthest, and then calculate how to step the other axis. However, this implies that the timebase of the motion is going to be that of the "master axis". This works fine if the ratios are even multiples, but when you end up with is steps at the same rate, with the other axis just missing steps. You can see what I mean here (to X0.3 Y.02 Z.01) :

[www.getdropbox.com]

what I would prefer to have is that all the axis are stepped at an even rate, rather than just having steps skipped. I'm not sure how to best implement it, or if it really matters. Any thoughts?

Hi

Homing.

On a conventional CNC you switch on.

Hit the home or reference button.

The machine travels to the limit switches (Usually plus in Z and Minus in X & Y).

It runs on to switch, backs off slowly and registers the point the switches change state. With good switches you can get .001" repeatability.

Sometimes this position is registered as XY & Z zero but often you can have a preset parameter so the zero position of the machine is where you want it.

It save an enormous amount of setting time and prevent errors. Some machine will not run until you reference.

I routinely switch my machine on, home the axes and just run a programme as I know exactly where the zero is set.

---

Regards

Ian

That's a great idea. I like it. What kind of switches give you the .001 accuracy?

I've been thinking about the stepping rate issue and how the code was all based on the timestep of a master axis. I totally rewrote it such that it was based on time, so all the axis will be able to step independently in much smaller timeslices. The new code algorithm was evaluated using this script to feed values into excel.

maxtime = 100.0; #duration of motion in microseconds
steps = 41.0; #number of y steps
timePERstep = maxtime/steps; #time per step

y=0;
stepped = 0;
oldTimeIntoSlice = 0;
for time in range(0,int(maxtime)):
        timeIntoSlice = (time%timePERstep);
        if (timeIntoSlice < oldTimeIntoSlice):
                stepped = 0;
        oldTimeIntoSlice = timeIntoSlice
        if (timeIntoSlice >= 0.5*timePERstep) and (stepped == 0):
                #print 'step'
                stepped = 1;
                y = y + 1;
        print '%d,%d' % (time,y)

output here (graph) :

[www.getdropbox.com]

I've also moved the firmware here :

[code.google.com]

Hi

My switches are simple V3 roller microswitches running on to a 45 degree ramp. I think the opto end stops might also be good.

We have used a precision (Euchner or Balluf V. expensive) micro switch and achieved .0002" repeatability on one machine.

---

Regards

Ian

I've made good progress on my new stepping algorithm, and will test it out shortly. I put together a writeup here with more details

[reza.net]

I think you're right and the limiters are more useful than the keypad interface, so I'll go ahead and put that back in there.

reza Wrote:
> PORTB = _BV(1) | _BV(2) | _BV(3); //set three bits
> high

Yeah, digitalWrite is slow.

You can use direct port writes but first you have to set the pinMode for the pin you want to write to. ie.

To use PORTD = PORTD & (B11101111);//pin 4 LOW
first set pinMode(4, OUTPUT);
or use "DDRD = DDRD | B11111100;" which sets 2 to 7 as outputs.
check out: http://www.arduino.cc/en/Reference/PortManipulation

Haven't tried 3 motors at a time yet but one at a time works great.

I like the way you're using timing instead of the breshenham method. I had the Breshenham method working...guess I'll rewrite with a timing method. Not for a while though. I'm sllllloooow!

I've some arduino code that runs a stepper with the timing set with a pot if you'd like to see it.(messy code. Trying to use a serial LCD to get the timing.) Using it to test my axis movement and determine how fast my y stage can move. (really, really slow too!)

Attached a picture of the sorta CNC-repStrap I'm building.(again slowly)

@reza...I'd like to discuss your python code a bit.(I understand it's just test code.)

Trying to add time slicing into the stepper test code I already have.

Q's:
What is "#duration of motion in microseconds" derived from?
Is "motion" the time required for one step of the stepper?
(ie. time for the step pin to be high plus the delay for the stage to physically move?)

Reading my own code is hard enough. Reading someone else's, in a language I have no experience in, is even harder! Thought I understood your's until I tried to implement it.

Sorry for not replying sooner, I'm not checking this thread often. As I've said, I've released the code on google.com (http://code.google.com/p/rsteppercontroller/) and your welcome to just copy it. I've not tried using the stepper controller modules I have, but if they operate as per their specifications, it should work great. I'm thinking of designing an arduino shield that has 3 of these ICs and some connectors to simplify the implementation for others, though I have no desire to sell them. If anyone is interested in dealing with the sale and supply side, I am happy to collaborate. I don't want any of the proceeds except that my name be associated with the final device.

@arvin I actually have a lot more experience coding for the AVR in C than I do using he arduino interface, but thanks for the pointers. Specifying bitfields that way is a bit ugly, imho. the macro _BV(x) maps to (1<<x) which is an easier way of setting bits. use REGISTER |= _BV(bit) to turn on and REGISTER &= ~_BV(bit) to turn off. These methods used to be included in WinAVR as sbi(port,pin) and cbi(port,pin). I find the sufficiently useful and sometimes reimplement them via

#define sbi(port,pin) port|=_BV(pin)
#define cbi(port,pin) port&=~_BV(pin)

-Reza

I just tried "_BV(1);" in arduino0017 and recompiled and got no error so I guess it should work. You said you couldn't use it. Did you get an error message or did it just not work as expected?

In the extended reference I see:
* lowByte()
* highByte()
* bitRead()
* bitWrite()
* bitSet()
* bitClear()
* bit()
bit() doesn't look like a #define. It looks like a function so would have memory impact, where a compiler directive would not, but gets the "value" of a bit.

Nope just but both in a sketch and neither one of them added to the size of the sketch so both are implemented as a define somewhere.

void setup(){
   Serial.begin(9600);
   delay(2000);
}

void loop(){
   int a = 0;
   Serial.print(a);
   //a = bit(1);
   //a=2;
   Serial.print(a);
   //a = _BV(1);
   a=2;
   Serial.print(a);
}

formatted it a little better.

Edited 2 time(s). Last edit at 10/11/2009 09:04AM by Arvin.

I've added support for limit switches to the code, updated in SVN. -r

Hi,

Really interested in this - your site seems rather devoid of notes but I tried uploading the latest version of rsteppercontroller. It verifies ok but when I try to upload it fails

Binary sketch size: 10498 bytes (of a 14336 byte maximum)

avrdude: stk500_getsync(): not in sync: resp=0x00
avrdude: stk500_disable(): protocol error, expect=0x14, resp=0x51

Bascially all I want is a g-code interpreter to use on my mill as bit bashing my parallel port no longer works

Thx.

Reza...can you give us a link to your code in svn? The link to google code has no downloads available. I've tried my code, and while it worked, I'm not happy about it. I'm almost ready to use my repstrap. (No extruder and z axis is acting up)

Would like to give your code a try.
Arvin

Hi JohnW

> avrdude: stk500_getsync(): not in sync: resp=0x00
> avrdude: stk500_disable(): protocol error, expect=0x14, resp=0x51

Looks like a comms issue. I go this when I downloaded a new version of gcc-arv
and stuffed up the dependencies.

Can you upload anything to the arduino?

I solved my problem by blowing the operating system away and starting again.

regards

Stephen

Hi Reza ()

I can't find your code.

Could you just upload/attach it to the forum?

regards

Stephen

You can find the latest version here: [code.google.com]

I've also noticed that on many gcode machines that extra spaces although should not be an issue, are. x1 y1 is different than x1 space space space y1 for example.

make sure that your output code starts with g1 and has only 1 space after it

g1 x1 y20

also get rid of all comments and commas

What happens with the two spaces? I'm having weird behaviour with my firmware and have double spaces. I have a script that can remove them but I want to know for sure that this is what is causing them and the problem only surfaces once in a while. I sometime send rapid E commands (5D firmware) and negative values will still extrude forward. This is after a while where it goes fine. The same file could print correctly or go terribly wrong. Is it related? If not, I don't want to hyjack this topic but maybe you have a pointer...

---

Regards,

Erik de Bruijn
[Ultimaker.com] - [blog.erikdebruijn.nl]