[LinuxCNC/linuxcnc PR#151] M98/M99 subprograms About GRBL HAL

Issue #151 | 状态: 已关闭 | 作者: zultron | 创建时间: 2016-08-19

(Originally introduced in emc-developers)

This patch implements subprograms as defined with a bare numbered
O-word, called with M98 and returned from with M99. This common
style of subprogram is used in Fanuc, Haas and other controllers.

Implementation

In essence, M98 and M99 are O-words. O-words are handled
specially compared to M- and G-words, both when being read
(e.g. parsed earlier in interp loop than other codes) and when being
executed (e.g. own translation unit with functions for searching and
seeking across g-code file blocks). Therefore, they are implemented
next to other O-word code, and where possible they share the same read
and execute flow. There is a major semantic difference compared to
traditional rs274ngc subroutines: Execution starts at the beginning of
the file and continues right into a program definition rather than
skipping past. Such a program definition at the beginning of the
file is the ‘main program’, although not treated specially by the
interpreter, and M98 may only call subprograms following the main
program.

In the interpreter’s read phase, M98 call and M99 return are
handled in reado() at the same time as O-words in interpread.cc.
M98 is handled much like O call.

During execution, M98 P1 is quite similar to O1 call, and they
share the Interp::executecall() function in interpo_word.cc.
Still, M98 must be distinguished with the special call_type = CTNGCM98SUB to signal to executecall() and unwind_call() that
parameters #1 through #30 are global. The call type is also used
to prevent programmers from mixing e.g. O sub with M98 and M99.

When M98 is followed by an L-word (repeat count), it maintains a
loop counter in the parent context m98loopcounter field, and
special logic in execute_call() manages call return block location
for iteration.

An M99 in the main program (top-level call stack) has a special
meaning. There is no subprogram to return from, so instead it means
to skip back to the beginning of the file and resume execution; in
other words, loop endlessly over the program. In this case, M99 is
treated like an M-word handled by read_m(), and shares modal group 4
and convert_stop() with M02 and M30.

The interpreter cannot handle endless loops because it would queue
canon commands indefinitely without a program end or a queue buster,
and never issue them to task for execution. Therefore the interpreter
distinguishes the following two subcases of the M99 endless loop:
– Task: During real execution of a program, an M99 in the top-level
context is treated like a queue-buster. The interpreter queue is
flushed to motion (after queuing any link segments) before looping
back to the program beginning. Task enables this special behavior
by calling interp.setlooponmainm99(true).
– SAI, preview, etc.: When not actually executing, an M99 in the
top-level context is treated like an M30, program stop. The
assumption is that a single loop will be sufficient for
non-execution use cases, such as to render a tool path preview or
check the canon commands generated by a program. This is the
default behavior, also explicitly set by calling
interp.setlooponmainm99(false).

Fanuc and rs274ngc sub call coexistence

The syntax differences create no parsing ambiguities. During
conversion the two styles share some code paths, where any differences
are handled by the O-word (or M-word) type and call style.

Additional checks ban mixing rs274ngc and Fanuc sub blocks within one
call/return cycle in hopes of reducing confusion arising from careless
mixing and the unintended side-effects of style differences.

If there is reason to disable Fanuc subroutines, do so by placing
DISABLEFANUCSTYLE_SUB = 1 in the [RS274NGC] section of the
.ini file.

#1 – zultron 于 2016-08-19

Ugh, things keep changing out from under this patch. I’ll fix it later….

#2 – zultron 于 2016-08-20

By the way, this patch depends on the fix in #150.

#3 – zultron 于 2016-08-20

Wow, thing really are moving around under me…. This time it’s 85210a33. Trying once more to get a passing build….

#4 – zultron 于 2016-08-20

Yay! A pass on Travis.

#5 – andypugh 于 2018-07-29

This has been hanging around for years. Do we currently think it is worth doing?
(meta-question, do we care how Fanuc do things?)

#6 – zultron 于 2018-07-29

Last I heard, @cradek didn’t want it because of fears about maintenance, and @mozmck briefly expressed interest.

I thought the real value was in using output from CAM programs that don’t have a special LCNC post-processor, but I haven’t heard much clamor for the feature.

This feature has been in the PathPilot tree since I wrote it.

Anyway, if there’s no interest in merging it, then just bin it.

#7 – andypugh 于 2018-07-29

I have a feeling that CAM programs almost never output subroutines anyway, neither in LinuxCNC nor Fanuc style. Half of them don’t even output curves…

#8 – KimK 于 2018-07-29

I would support this feature because of its popularity in other controls. No reason to give a potential user/company a reason to reject LinuxCNC if we don’t have to. And the patch includes DISABLEFANUCSTYLE_SUB so that’s available if needed.

#9 – KimK 于 2018-07-30

Oops! Didn’t mean to thumbs-up my own comment, I just wanted further information.

#10 – rene-dev 于 2018-07-30

if its in PathPilot, it should be stable I guess? any good reason not to merge it?

#11 – andypugh 于 2018-07-30

I guess it can only ever change existing behaviour if the G-codes are actually used. I think that the risk is small and the benefit is small, but bigger.

#12 – zultron 于 2018-07-30

I’m happy to fix the conflicts and update the PR if this will get merged. In that case, I may have one bug fix in my tree that needs to be picked over, too. Let me know.

I did say it’s ok to close the PR if there’s insufficient interest (and that still stands), but don’t take that as loss of interest on my part: I do still hope that this can be merged.

#13 – c-morley 于 2018-07-30

well since I see nobody refusing it, seem to have a maintainer, it’s in pilotpath with no apparent problems, and master is has no signs of being released soon, I see no reason to leave this out at this time.
My 2 cents

#14 – zultron 于 2018-07-30

@c-morley, thanks for the +1.

I’m happy to be the maintainer. In order to do better in that regard, I just set the GitHub tracker prefs to “Watching” so that I get updates for all issues in my inbox.

#15 – samcoinc 于 2018-07-30

I think this would be a good addition also.

On Mon, Jul 30, 2018 at 2:07 PM, John Morris
wrote:

> @c-morley <https://github.com/c-morley>, thanks for the +1.
>
> I’m happy to be the maintainer. In order to do better in that regard, I
> just set the GitHub tracker prefs to “Watching” so that I get updates for
> all issues in my inbox.
>
> —
> You are receiving this because you are subscribed to this thread.
> Reply to this email directly, view it on GitHub
> <https://github.com/LinuxCNC/linuxcnc/pull/151#issuecomment-408976110>,
> or mute the thread
> <https://github.com/notifications/unsubscribe-auth/AQ8TQtwoovMH0KnB9AyjRdGRH-Zw9upBks5uL1mJgaJpZM4Joy-w>
> .
>

#16 – mozmck 于 2018-07-30

I am still interested and would probably use sometimes for looping a program. Since we have a maintainer I would think that would help with @cradek’s worry. The only issue I see currently is that there is a conflict preventing merging with master. I started to look at it and ran out of time – but it looked like it shouldn’t be too hard to resolve.

#17 – KimK 于 2018-07-30

The only thing I noticed was that in
tests/interp/subs-follow-main/expected
and in
tests/interp/m98m99/(various places)
that whenever
N….. SETG5XOFFSET(1, 0.0000, 0.0000, 0.0000, 0.0000, 0.0000, 0.0000)
or
N….. SETG92OFFSET(0.0000, 0.0000, 0.0000, 0.0000, 0.0000, 0.0000)
were mentioned, they seemed to be initializing with six values (XYZABC)
instead of the anticipated nine (XYZABCUVW).
But maybe tests/ only works with the original six values?
If so, maybe a bug should be filed on tests/ ?
I’ll leave this to the experts to sort out.

#18 – zultron 于 2018-07-30

@KimK, the SETG*OFFSET canon commands do indeed include the UVW axes. That output is from the stand-alone interpreter, which is mainly used for testing purposes, and the author must’ve thought that showing the extra axes in the output wasn’t valuable. The axes are still being passed around behind the scenes, though, and are presumably honored by the interpreter used by task. See these lines:

https://github.com/LinuxCNC/linuxcnc/blob/master/src/emc/sai/saicanon.cc#L248
https://github.com/LinuxCNC/linuxcnc/blob/master/src/emc/sai/saicanon.cc#L226-L227

In any case, this PR doesn’t change anything in that area of the code.

Thanks for looking at the code!

#19 – KimK 于 2018-07-30

OK John, excellent, thanks for explaining that. Well, we’ll look forward to hearing when it’s ready to merge, then? And this is going into master, can it be cherry-picked to 2.7? PathPilot is based on 2.7 now, right?

#20 – zultron 于 2018-07-30

The current patchset is based on LCNC 2.7, but it’s up to maintainers to decide on any merge’s target branch. Forward-merging should be trivial, unless master has any other large changes in interp that I’m unaware of.

As @KimK stated above, the feature could be disabled by default in configs. That’s an option maintainers may choose, and which may play into the choice of target branches.

#21 – andypugh 于 2018-07-30

I think that as a new feature rather than bugfix it should go in 2.8.

#22 – zultron 于 2018-07-31

Ok, so I did a bunch of fancy rebasing, and force-pushed a new commit.

The original patch and a few fixes are rebased onto the v2.7.14 tag. This could be cleanly merged into the 2.7 branch if desired.

That result is merged into current master with all conflicts resolved. The v2.7.14 base allowed a clean merge with only the M98/M99-related changes, and without dragging in all the 2.7 commits not yet in master.

The PR can be merged into master as-is, or if 2.7 is the better target, just pop off the top commit.

#23 – zultron 于 2018-08-25

What’s left to do before this PR can be merged? It has extensive unit tests, significant testing, interest from several in the community, a maintainer (me), and no identified issues.

Can a LinuxCNC GitHub organization member please comment here? Thanks!

#24 – c-morley 于 2018-08-27

If there are no objections by next weekend, i will merge this.

#25 – zultron 于 2018-08-27

Thanks, @c-morley !

#26 – mozmck 于 2018-08-27

I say go ahead!

#27 – c-morley 于 2018-09-02

Thank you for your work, pledging to maintain it and patience on merging it.

#28 – zultron 于 2018-09-02

Thanks, @c-morley. I’ll make good on that pledge.

原始Issue: https://github.com/LinuxCNC/linuxcnc/pull/151

Implementation

Fanuc and rs274ngc sub call coexistence

评论 (28)