"Failed to find ... before EOF" with %-delimited subroutine if loaded G-code file is also %-delimited

[andypugh]

Here are the steps I follow to reproduce the issue:

1) Create a (named?) subroutine file that is %-delimited 2) Load a G-code file that is also %-delimited 3) Attempt to run the subroutine.

This is what I expected to happen:

The subroutine should run

This is what happened instead:

An error message "Failed to find before EOF"

It worked properly before this:

24ef2feae

Information about my hardware and software:

• I am running ... master / run-in-place

[andypugh]

Forum: https://forum.linuxcnc.org/40-subroutines-and-ngcgui/36030-call-external-subroutine-not-working-in-recent-versions-of-lcnc?start=10#126297

Mailing list (git bisect): https://sourceforge.net/p/emc/mailman/emc-developers/thread/CAN1%2BYZV8nr5ZsCWQxbYWtZADhk-w5_cUKo%3DNCcno19kOxLWJzA%40mail.gmail.com/#msg36585666

[zultron]

Thanks for the heads-up, @andypugh, I'll take a look.

[zultron]

Well, bad news. I'm finding that before 24ef2fe, in a %-delimited program, calling a %-delimited sub also closes out the subroutine file from the first % and the subroutine is never run. The difference is that the program silently exits at that point, rather than printing the confusing message and then exiting.

[zultron]

Here's my test case (GH doesn't allow .tgz files). I run it like this, from the top-level of a linuxcnc RIP build tree:

unzip percent-subs-test.zip 
(cd tests/interp/percent-subs/; ./test.sh)

[andypugh]

Well, that's interesting. Time for another Git Bisect then, I guess.

[zultron]

I think we first need to decide whether %-delimited subs should be allowed in the first place. I have no opinion about this, personally, except that given that this hasn't been reported earlier, it sounds like almost nobody begins their sub files with %, and I'd prefer the simplest fix that gives reasonable behavior.

If the answer is %-delimited subs are NOT allowed, then we can give a more meaningful error message when in an open subroutine file. (This is a simple fix.)

If the answer is that they ARE allowed, then we could reuse code from Interp::open() that skips the initial %, as explained here, over in Interp::read_text() around here.

[andypugh]

I just checked out ad6c9b5c7 and found that a (simple) %-terminated sub and %-terminated G-code file seemed to work with the sub called via an MDI_COMMAND. I do see the silent-ignore of the sub with the above hash if the sub is called from the G-code. (The original problem on the forum was a subroutine tied to an MDI_COMMAND that worked, or not, depending on the delimiter of the loaded G-code file.)

Latest master gives the "sub not found before EOF" in both scenarios and from the MDI window.

I am not sure who would opinionate on the matter of what should be allowed. The usual suspects have gone very quiet.

As far as I can tell from a quick Google % originally marked the start of data, so one could argue that it is not sensible to have it in a subroutine file which is logically inserted in the middle of the main file being executed.

[shipmodeller]

I would like to put my 2 cents inhere. Since a file subroutine is a program, then it could and should be treated as a nested XML type of structure. If the file loading program detects a beginning %, then it should expect a trailing %, This should be fairly easy to implement. If you disallow the leading % in a subroutine, you are saying there are two rules to write code, one for mainstream code, one for included code. If you disallow the leading % in a subroutine, then you should also disallow through appropriate M2 after the return in a subroutine, or for that matter, ANY code after the return that is not a subroutine in a called subroutine file. Consistency is key.

[shipmodeller]

Put two files here.. One is testcall.ngc, the other is testexternal.ngc. Both extremely simple. In the testcall program.. I have a subroutine that is also called within the file. So in this case, I use the % to delimit the entire file, and it runs very nicely. In the second, testexternal, I am calling the subroutine in the first file, but it fails because of the percents. So... if I wanted to be correct... I would have to have generated a 3rd file, ( yet another to maintain.. ) that contains ONLY the subroutine. But since LinuxCNC allows me to put other code in the same file as the callable external subroutine.. I feel that either LinuxCNC would have to test and report that external code is in the file, and even so far as verifying that an external called program does have correct formation. It shouldn't be that hard to either generate an error message that makes better sense than what we have today, or allow the % in and out. of the file. If the % is not allowed, than I would also have to argue that the m2 should be disallowed in a subroutine file. 2 cents got multiplied out to a quarter, sorry.. but when I was constructing my subroutines.. the EXAMPLES that I modeled them from contained %, and since they use to work in the older version of LinuxCNC.. I thought I was golden.. Thanks for listening.

nc_files.zip

[zultron]

First, let me restate that I'm not a G-code expert, and my usual go-to, Smid, is packed up in a storage cubicle ATM.

I would like to put my 2 cents inhere. Since a file subroutine is a program, then it could and should be treated as a nested XML type of structure.

Is it true that a file subroutine is a program? This is counter-intuitive to me, since all the file subs I've seen (95% from the LCNC source tree) contain nothing outside of the o<mysub> sub...o<mysub> endsub that would execute if run as the primary program file. (I'm not counting files such as tests/remap/variable-injection/rm405.ngc that follow the sub with m2\n%, which appears completely extraneous to me, and in any case don't call the defined sub from that file.) It's also counter-intuitive since in LCNC, sub files are relegated to their own directory in the .ini file, a further indication that they're not intended to be run stand-alone as a main program. I can imagine value in a file doubling as a sub or as a stand-alone main program, but even while this is theoretically possible, from what I seen all indication points to that being uncommon practice.

If the file loading program detects a beginning %, then it should expect a trailing %, This should be fairly easy to implement. If you disallow the leading % in a subroutine, you are saying there are two rules to write code, one for mainstream code, one for included code. If you disallow the leading % in a subroutine, then you should also disallow through appropriate M2 after the return in a subroutine, or for that matter, ANY code after the return that is not a subroutine in a called subroutine file. Consistency is key.

I agree that consistency is key. The leading % is not expressly disallowed in the subroutine. Rather it's just what you say: there are two functions that read files, one for the main program, one for subroutines. In https://github.com/LinuxCNC/linuxcnc/issues/559#issuecomment-463718696, I pointed out the locations of both functions. Please check them out. Remember the rs274ngc G-code interpreter (and everything else under the src/emc/ directory) is one massive pile of inconsistencies, arising from dozens of contributors over decades adding new, large features, but without clear guidelines on organization and architecture, inevitably leading to problems like this.

Again, I don't have a set idea about what's the correct behavior for a G-code interpreter, but I do think that whatever we choose should be the simplest fix to produce reasonable behavior. I'd want more convincing evidence that allowing %-delimited sub files is valuable before I'd favor implementing that fix, assuming I'm the one eventually to do it.

[shipmodeller]

"Allowing" ... This was the way it worked in the previous version, and no warning or manual entry for the new version indicated that LinuxCNC was deprecating a behavior that it allowed before. So, you are right.. Also, there are relatively few users of subroutines, and very few posts actually generate them. So, yes, it is an outside case, maybe. If you took a poll.. you might find few.. generally relegated to being linked with buttons. Personally, we have two issues here.

One: Does LinuxCNC want to fix the % issue in subroutines, or just put an error message that tells exactly what is wrong instead of the obtuse message that is in there now. That would work for me.. though that means not supporting backwards compatibility without warning.

Two: Should LinuxCNC allow ANY other ACTIVE code in the subroutine file other than that which is within the constraints of the subroutine itself. ( Active means other than comments. A well documented subroutine file should have verbose comments.. ) That would prevent me from doing code with subroutine.. which I like to do for testing purposes, etc...

I'm ok with either or both... I've been a programmer for more than 50+ years, so I understand the issue.. but consistency and definition is key. Fanuc subprogram calls are different than LinuxCNC subprogram calls. I was looking for some examples from other CNC makers.. but I didn't find anything similar. MOst users seem to use subroutines internal to programs, and don't call external file subs, except to attach to buttons. I am sure ya'll will arrive at a conclusion that works for the betterment of LinuxCNC. Thanks!

[andypugh]

Actually, the previous versions of LinuxCNC might have "allowed" the syntax, in that no error message was given. But under the same conditions the result was that the subroutine did not run at all, but with no warning. Which is probably worse.