Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

protocol_enqueue_gcode does not set last_error #488

Open
Nick507 opened this issue Apr 8, 2024 · 3 comments
Open

protocol_enqueue_gcode does not set last_error #488

Nick507 opened this issue Apr 8, 2024 · 3 comments

Comments

@Nick507
Copy link

Nick507 commented Apr 8, 2024

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
Copy link
Contributor

terjeio commented Apr 8, 2024

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
Copy link
Author

Nick507 commented Apr 8, 2024

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
Copy link
Contributor

terjeio commented Apr 8, 2024

... 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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants