search

grblHAL / core

grblHAL core code and master Wiki
Other
304 stars 73 forks source link

protocol_enqueue_gcode does not set last_error #488

Open Nick507 opened 2 months ago

Nick507 commented 2 months ago

Hello Terje,

Was it intended to not set last_error in case of extra command handling - initiated from protocol_enqueue_gcode? I've fixed it this way: image

I'm working on my lathe panel plugin, and found that it is not possible to track command execution result. I started from hook hal stream, but found this way a bit tricky in case of multiple hooks from another plugins. So I decided to use protocol_enqueue_gcode, but didn't find a way to handle its result without proposed change.

terjeio commented 2 months ago

Was it intended to not set last_error in case of extra command handling - initiated from protocol_enqueue_gcode?

Yes, commands coming that way should be from tested code - it should not (and can not) be used to stream general gcode. And if an error status is to be set it should be in a different variable in order to not interfere with the main stream?

I started from hook hal stream, but found this way a bit tricky in case of multiple hooks from another plugins.

What was tricky?

Nick507 commented 2 months ago

Yes, commands coming that way should be from tested code - it should not (and can not) be used to stream general gcode.

You are right, I'm using it for hard-coded correct commands, but is it impossible that command will fail because of some reason in grbl module - some state caused by previously executed commands?

What was tricky?

For example, from macros.c file:

if(hal.stream.read != file_read) {
...
        hal.stream.read = file_read;

if another plugin hooks stream later, but will behave like proxy - pass data to previous stream function, on next call macros plugin will hook stream twice. I don't want to say that it is unsolvable task, just thought that protocol_enqueue_gcode is more correct way.

terjeio commented 2 months ago

... is it impossible that command will fail because of some reason in grbl module - some state caused by previously executed commands?

IIRC not possible, if a command leaves the machine in a state that will cause the next to fail it will raise an alarm. Legacy Grbl is IMO a bit dangerous in that it will accept and execute new commands from the input stream after an error has been detected, I added the last error check in protocol.c to disallow that. IMO this is important when the sender fills the stream input buffer and thus cannot always react in time to error returns before further blocks (lines) is sent to the parser.

Hmm, when I think about it perhaps protocol_enqueue_gcode() should not accept gcode if last_error is set?

just thought that protocol_enqueue_gcode is more correct way.

It is for single commands (or blocks) - if you want to execute a number of commands then it is, again IMO, better to temporarily claim the input stream in order to ensure that they are not mingled with unwanted commands.