jminardi / mecode

GCode for all
MIT License
221 stars 61 forks source link

Fix https://github.com/jminardi/mecode/issues/63 and https://github.com/jminardi/mecode/issues/64 #65

Closed fake-name closed 6 years ago

fake-name commented 6 years ago

The title.

jminardi commented 6 years ago

Other than the breaking change, this PR looks good to me.

fake-name commented 6 years ago

That's reasonable. I'll admit that changing the default was definitely a change made in anger.

I'm currently running into issues with the arc functions:

self.g.arc(x=tgt_x, y=tgt_y, z=None, radius=radius, direction="CCW", helix_dim='z', helix_len=helix_z_step)

produces

G16 X Y z ;coordinate axis assignment
G17 ;XY plane
G3 X772.542486 Y-2377.641291 R2500.000000 G1 Z45.4545454545

which both breaks linuxcnc, and doesn't make sense in the first place. Everything I can find indicates that G16 doesn't take any parameters.

fake-name commented 6 years ago

Oh wow:


    def arc(self, x=None, y=None, z=None, direction='CW', radius='auto',
            helix_dim=None, helix_len=0, **kwargs):
        <snip>
        dims = dict(kwargs)
        if x is not None:
            dims['x'] = x
        if y is not None:
            dims['y'] = y
        if z is not None:
            dims['z'] = z
        msg = 'Must specify two of x, y, or z.'
        if len(dims) != 2:
            raise RuntimeError(msg)

        <snip>

        values = [v for v in dims.values()]

        <snip>

            k = [ky for ky in dims.keys()]
            cp = self._current_position
            dist = math.sqrt(
                (cp[k[0]] - values[0]) ** 2 + (cp[k[1]] - values[1]) ** 2
            )

How did this ever work? Dicts are not ordered, so you'd randomly swap the x and y components of the current/target position when computing absolute moves.

jminardi commented 6 years ago

Are you trying to actually execute a helix? (i.e., arcing through 2 dimensions while linearly moving through a third orthogonal dimension)

If not, you do not need to pass the helix dimension.

If so, I need more information on how linux CNC expects helixes to be defined. From what I understand a helix is not part of the standard gcode spec, but something that individual machine integrators need to invent for themselves. Aerotech expects the G16 before executing a helix to know which are the arc axes and which is the linear axis.

If linuxCNC expects something substantially different, there would need to be some work to implement a gcode "flavor" selector that would make this task easier.

fake-name commented 6 years ago

Yeah, I'm trying to do actual helixes (note that I haven't implemented rotations yet, and I need to).

Basically, if I remove the G16 stuff, it works:

G3 X2500.000000 Y-0.000000 I-2165.063509 J1250.000000 Z4537.815126
M63 P2
G3 X2165.063509 Y1250.000000 I-2500.000000 J0.000000 Z4579.831933
M62 P2
G3 X1250.000000 Y2165.063509 I-2165.063509 J-1250.000000 Z4621.848739
M63 P2
G3 X0.000000 Y2500.000000 I-1250.000000 J-2165.063509 Z4663.865546
M62 P2
G3 X-1250.000000 Y2165.063509 I-0.000000 J-2500.000000 Z4705.882353
M63 P2
G3 X-2165.063509 Y1250.000000 I1250.000000 J-2165.063509 Z4747.899160
M62 P2
G3 X-2500.000000 Y0.000000 I2165.063509 J-1250.000000 Z4789.915966

I'm also generating IJK arcs, rather then radius arcs, but that's a minor difference.

A brief survey of G2/G3 documentation leads me to believe that in general, the linear axis is basically perpendicular to the arc plane, and it's not configurable. This appears to be correct for Mach 3/4, LinuxCNC, Haas controllers, and a bunch of other controls.

jminardi commented 6 years ago

Regarding your second comment, the values and k arrays match up one to one as they are both created from the dims dict.

values = [v for v in dims.values()] k = [ky for ky in dims.keys()]

the distance is just then calculating the sqrt of the sum of the squares.

I might be missing something but there doesn't seem to be a bug there. If you can capture a failing example though we can fix it and write a test for that case.

fake-name commented 6 years ago

Dicts are not ordered. Iterating over them multiple times makes no guarantees about the order the contents will be returned.

Functionally they're ordered, but that's an implementation detail that you should not rely on, and other interpreters (e.g. not cpython) could happily randomize the iteration order every time and still be correct.

If you rely on the ordering of keys() or values(), you have to convert to a list, and sort first (or use an ordereddict).

jminardi commented 6 years ago

The documentation seems to contradict that. You can rely on the same ordering when accessing an ordered list (Not a specific ordering, but the ordering is preserved)

https://docs.python.org/2/library/stdtypes.html#dict.items

It might be different for Py3.

fake-name commented 6 years ago

Well, uh, huh. I'm wrong.

Sorry about that.

fake-name commented 6 years ago

Also, it looks like the aerotech stuff is the odd-one out with the arc stuff.

There are lots of extensions, but the core movement features have been pretty nailed down for a while. I wouldn't expect a general library to support linuxcnc's cubic splines or nurbs, but the basic stuff should match.

See the NIST g-code standard: https://ws680.nist.gov/publication/get_pdf.cfm?pub_id=823374

(I'm doing some oddball stuff, basically I have to trace a helix across the surface of some primitives (a cylinder and a hemisphere), and I'm attempting to do so without resorting to just generating a crapload of linear moves, so I'm fiddling with the internal stuff a bunch.)

fake-name commented 6 years ago

Rolled back the aerotech defaults thing.

I've also added the ability to generate IJK arcs, rather then radius arcs.

Next-up: cubic splines. Never mind, apparently cubic splines cannot be generated in 3d. I just went with linear interpolation.

jminardi commented 6 years ago

LGTM 👍

fake-name commented 6 years ago

Woot!