write_gcode fails when run from shellscript

[Jørn Sandvik Nilsson]

When running write_gcode in a script it failes with the message below

#!

> open_gerber gerber.gbr
isolate gerber.gbr
cncjob gerber.gbr_iso
write_gcode gerber.gbr_iso_cnc gerber.gcode

Operation failed: 'NoneType' object has no attribute 'export_gcode'

This happens when run from script file, or pasted as 4 lines into the cli. When pasting the 4th separately, it works as expected.

I'm guessing that some part of "cncjob" is done async, and is not finished when write_gcode runs.

I am attaching the gerber, although i don't think this is a corner case.

[Jørn Sandvik Nilsson]

I tried spawning a delayed job for write_gcode, and it worked. The function is horribly ugly and by no means a solution, but it is another clue suggesting it's a synchronization problem. Here's the modified write_gcode function:

        def write_gcode(obj_name, filename, preamble='', postamble=''):

            # To be run in separate thread
            def job_thread(self, obj_name, filename, preamble, postamble):
                time.sleep(1)
                try:
                    obj = self.collection.get_by_name(str(obj_name))
                except:
                    return "Could not retrieve object: %s" % obj_name
                try:
                    obj.export_gcode(str(filename), str(preamble), str(postamble))
                except Exception, e:
                    return "Operation failed: %s" % str(e)

            # Send to worker
            self.worker_task.emit({'fcn': job_thread, 'params': [self, obj_name, filename, preamble, postamble]})

[Jørn Sandvik Nilsson]

Digging further, I couldn't find any mechanisms FlatCAMObj.generatecncjob() could use to synchronize with the FlatCAMWorker job. There' probably a reason for the backgrounding of this job as well, so synchronization before generatecncjob returns is probably not the way to go anyway.

A better solution would be to query the FlatCAMWorker from the shellscript eval loop, and not continue until all background jobs are finished (or timeout at some point). That would fix the issue for script running at boot time, but not when pasting multiple lines in the cli window (although the samme logic could be added there).

I can implement this and issue a pull request if you are ok with the idea. I am not very familiar with the rest of the code yet, so you might have other and better ideas on how to attack this.

[Juan Pablo Caram]

Hi Jorn,

I believe you are correct about the CNCJob still running in the background when requesting a write_gcode.

Perhaps a clean solution would be to keep a list of background jobs (If I remember well there is only one worker thread), with the "expected" output object. Then, if a new command wants to act upon an object that doesn't yet exists but is on the list of "expected" objects, then it waits or is added to the background tasks.

Whenever a background job is launched, the "expected" product is added to the list. As soon st the product becomes available, the item in the list is cleared.

If you like this approach and would like to implement it, I can guide you through the code. I think this should be relatively simple and add a lot of value.

[Jørn Sandvik Nilsson]

If we make the new command wait until the object is available, we'll probably block the main thread in some cases. I'm guessing the reason for the backgrounding of jobs is to avoid this, so pushing jobs to the back of the queue sound like a better way to go.

If we decide to do it that way, we should eventually make all commands "backgroundable", as they may depend on each other. This isn't necessary a bad thing, but it'll be a big change to many commands and might require extensive testing. I can start by implementing the "product queue" and porting write_gcode to use it.

Another approach would be to not do jobs in the background while executing scripts. This way cncjob would run in the main thread on boot, but in the background when triggered from UI. FlatCAMGeometry.generatecncjob() would need a new parameter (run_in_background=True) that can be toggled by the script parser. This will certainly be quicker fix, but might not be as extendable or elegant as the first.

Which approach do you like?

[Juan Pablo Caram]

I wonder why when doing just

open_gerber gerber.gbr
isolate gerber.gbr

the isolate command doesn't break in the same way as

cncjob gerber.gbr_iso
write_gcode gerber.gbr_iso_cnc gerber.gcode

does.

If I remember well, all these tasks are backgrounded. So the following one should always fail. I think this is worth investigating.

Regarding the two possible approaches, the run_in_background option seems cleaner. But will freeze the GUI, including the Shell, while they run. This could be bad. It would still be nice having the option in the function...

The "product queue" or "promise queue" (In javascript they use the term "promise" to refer to something that is running asynchronously and is expected to produce an object, never mind the name), seem a bit more complicated but should fully work.

This could be done cleanly with signals. Could add a 3rd parameter to the worker_task signal indicating the "promised" object:

self.worker_task.emit(function_name, params, promise)

And have a listener that automatically adds the promise to the list.

Then, there is a signal already there that announces that an object has been created. We would listen to that one to remove the promise from the list.

Then, when a function is called expecting to find an item it could do something like this:

def do_something_with(obj):
   if (object not in collection and object in promises):
      self.worker_task(...)
      return

   # The original code goes here

I'd be careful with the names of the promises because these are automatically changed appending _1 if the name exists already. Maybe with this new mechanism, the names can be determined ahead of time, because they will be reserved in the queue immediately, before they exist.

[Juan Pablo Caram]

I have a solution in place. It covers this case but it still has to be fully deployed.

Before generating an object in the background we create a promise:

# Create a promise with the name
self.app.collection.promise(outname)

# Send to worker
self.app.worker_task.emit({'fcn': job_thread, 'params': [self.app]})

Then, when a command-line command need an object it simply checks if there are any promises. If so, it sends itself to the background:

def write_gcode(obj_name, filename, preamble='', postamble=''):
    if self.collection.has_promises():
        self.log.debug("Collection has promises. write_gcode() queued.")
        self.worker_task.emit({
            'fcn': write_gcode,
            'params': [obj_name, filename, preamble, postamble]
        })
        return

    # The rest of the function follows

This looks risky. Might send itself an unlimited amount of times to the worker thread, but in fact that should never happen. If there is a promise, then an objects is supposedly being cooked in the worker, so the function shouldn't run again until the previous task in the worker completes, which should clear the promise (or promises), and make the awaited object available.

I tested this with the case in the bug report. @joernsn, can you test it? It was fixed in commit 0077aae.

[Klemen Zivkovic]

Tried with master and issue still persist.

[Juan Pablo Caram]

Please add some detail on what you precisely tried.

[Klemen Zivkovic]

I tried script in following folder (gerber file included) FlatcamSampleShellScript I have noticed some tasks run in parallel, and exporting gcode fails. WOuld it be possible to somehow mark asynchronous execution of commands from shell? Maybe another parameter like asynchronous=true near every command?

[Juan Pablo Caram]

Summarizing:

$ python "/home/kz/git/flatcam/FlatCAM.py" --shellfile=/home/kz/Dropbox/ESP8216_PROTO/flatCamScript.txt

flatCamScript.txt:

new
open_gerber /home/kz/Dropbox/ESP8216_PROTO/TopCopper.gbr -outname MyGerber
isolate MyGerber -dia 0.2 -passes 2 -overlap 0.05
cncjob MyGerber_iso1 -z_cut 0 -z_move 0 -feedrate 600 -outname TopCopperJob1
write_gcode TopCopperJob1 /home/kz/Dropbox/ESP8216_PROTO/TopCopperJob1.gcode
cncjob MyGerber_iso2 -z_cut 0 -z_move 0 -feedrate 600 -outname TopCopperJob2
write_gcode TopCopperJob2 /home/kz/Dropbox/ESP8216_PROTO/TopCopperJob2.gcode
open_excellon /home/kz/Dropbox/ESP8216_PROTO/Drill.drl -outname drill
drillcncjob drill -tools 1,3 -drillz 0.0 -travelz 0.0 -feedrate 200 -outname drillcnc
write_gcode drillcnc /home/kz/Dropbox/ESP8216_PROTO/drill.gcode
plot

You say "I have noticed some tasks run in parallel". Could you please provide more detail on how you observed this?

[Juan Pablo Caram]

The initial test case is not working for me ay more. The gcode files are not being written, but there are no errors. I will have to dig deeper for this one. Any help is appreciated.

[Juan Pablo Caram]

I deleted the previous comment by @zhivkec because it was too long. Please attach a file next time.

[Juan Pablo Caram]

Wait for object availability by subscribing to signal. Fixes #145.

→ <<cset 9724f3055191>>

[Juan Pablo Caram]

I changed the way I was approaching the problem. When write_gcode() determines that there are "promises", it subscribes itself to the new_object_available signal. So it will only try again when a new object has been created (potentially clearing the last promise).

I tested it by running the shell script flatCam.sh and by copying the contents of flatCamScript.txt directly into the FlatCAM shell. I had removed the part about the excellon file because I don't have it, but it is not related, or is it?. The two gcode files for isolation routing are created without problem.