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

[BUG] Is using inject_P() to send commands to the command buffer from the anycubic_TFT.cpp file actually the right way to do this #6

Open
hub-jba opened this issue Dec 27, 2019 · 4 comments

Comments

@hub-jba
Copy link

hub-jba commented Dec 27, 2019

Bug Description

Understanding that issue #4 will actually make the anycubic_TFT.cpp code command queuing work the same way in Marlin 2.0 as it did in Marlin 1.1.9 it's still worth considering if this is actually the correct way to do this.

Anytime we are injecting commands to the queue (aborting the current queue) we are technically throwing away commands that had been queued in the command queue just prior to our call.

This means that even in the Marlin 1.1.9 code version of the filament runout code exhibits this behaviour. And effectively once the print resumes we could in theory lose whatever commands were in the queue. Granted this is probably a small number of commands but still possible.

What we really need is a way to prevent the queue from accepting any more commands, but still enqueue our commands immediately after the last command in the queue.

Note sure if Marlin has a concept to do this. We need to review the key code to understand if this is even a problem, and if there is even an elegant way to solve this.

@github-actions
Copy link

github-actions bot commented Jul 6, 2020

This issue is stale because it has been open 30 days with no activity. Remove stale label / comment or this will be closed in 5 days.

@davidramiro
Copy link
Owner

Sorry, this should not have been closed. Some GitHub workflows were pulled from upstream that are a bit too quick for this repo.

I have merged your ExtUI repo code onto a new branch of this repo. CI reports that it wouldn't compile yet, will look into that later on.

@hub-jba
Copy link
Author

hub-jba commented Jul 12, 2020

I have it compiling and working with absolute minimal changes to actual bugfix2.0 branch of Marlin.

Currently it's one change in the m125 code to wait for confirmation if using extensible UI.

I am still tweaking the pause/timeout/resume behaviour as well as the filament runout behaviour, but everything else is working fine.

@hub-jba
Copy link
Author

hub-jba commented Jul 15, 2020

On this particular issue much of the marlin code uses inject_P to inject commands around the pause functionality so i think it's okay to close this. There are also enqueue and enqueue_now methods, but since the rest of the code base uses inject_P we should be good.

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