-
Notifications
You must be signed in to change notification settings - Fork 7
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
Feature/buttons #28
Feature/buttons #28
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks fantastic 👍 👍 I'm deep in the middle of shipping issues right now so I am going to hold off on merging until after this week's update because I won't have a chance to test on hardware today
@@ -235,6 +235,10 @@ sendMove = function (cmd) { | |||
'Z+': function () { | |||
jog({ Z: distance }) | |||
}, | |||
'Z_TOP': function () { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍 👍
//Disable_interface(true); | ||
//console.log("No heart beat for more than 20s"); | ||
//} | ||
if ((Date.now() - last_ping) > 20000){ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice! 20 seconds seems long, should we do 10 like the old one was?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I just used what was in the base code that was commented... I guess 10 would be ok too. the difference between this and the messaging is that this causes an action where the heartbeat you have just reports in serial...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That makes sense
www/js/tablet.js
Outdated
|
||
function togglePlayPause(fromPlay) { | ||
if (fromPlay) { | ||
if (STATE !== 'Idle' && STATE !== 'Hold') { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What happens in the hold state? Do the motors continue to hold position? We might want to test to make sure that is safe for them to do because I think maybe they can draw a lot of current without rotating enough to cool themselves that way
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think so? I know that the pauseGcode "!" call puts the machine in this state. From what I see, the machine is frozen. I think the idea of "pause" is that you'd want to continue soon, but I can run this longer and see what it does.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm a little worried about folks pausing their cut to come back the next day or something
I think that would be fine.
I removed that (yea, I know that was my initial reason for this branch to start with). My reasoning is that I don't think we need estop in software... I agree with dlang on this I think. It does not really work any different than "stop" except that the user has to reboot after hitting it, which if something urgent is happening the software probably isn't working anyway, so pullng plug / hitting physical estop is probably what we want. This is also your call on whether we should call estop or just stop. I think stop because you can recover from it...
I have had this happen from time to time, even before this change. I have not seen it for some time. I think there is an issue where the sd card is not ready for our call to get the files and it might be good to add a "refresh" button to the side of the dropdown, or bump that setTimeout up a bit on the init function.... if you navigate to the fluid first tab do you see files, and if you go back to the maslow tab do you see files? |
Lets hold on this... I found an issue where I got it stuck in the "hold" state. I'd like to simplify / rework this button handler. |
I'm seeing strange interactions on the play/pause/stop buttons, so I'll open a new one when I have them working better that said if you want to take the button you like and anything else, feel free, but I'd stay away from the play/pause |
Will do! I certainly want to grab those button icons |
OK, I reverted my button handling code and did a couple tweaks / renames to the old code. I ran this through some paces and I like the way it behaves. The play and pause buttons are now always there again and seem to do the right thing, and this logic was not being called. I'm thinking when you changed to canvases for the buttons, this logic was just not called any more, so I hooked it back up and it worked great. |
This looks fantastic! I was able to find a couple issues where the stop button wouldn't stop the cut immediately, it would still finish executing some more lines of code (probably what is in the buffer) before stopping. I did also have one case where pressing the stop button caused the machine to crash and required two restarts to get back to loading properly (the first time it didn't read the configuration file which is normal for fluidnc when it crashes), but I think those issues are not caused by this PR and are things that we can fix going forwards. Splendid work! 😀👍👍 |
I wasn't able to replicate the issues I was having before with uploading gcode files so no idea what that was 🤷♂️ |
@@ -376,22 +389,7 @@ function tabletShowMessage(msg, collecting) { | |||
initialGuess.br.x = parseFloat(msg.substring(13, msg.length)) | |||
return; | |||
} | |||
if (msg.startsWith('$/Maslow_tlZ=')) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I missed this at first and just found it while working on something else, but it looks like these got deleted by accident
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yikes, yes, I certainly didn't want that to be removed. Good catch!
@BarbourSmith
See thread https://forums.maslowcnc.com/t/m4-esp32-ui-for-stop-vs-abort/20932 (and this release
I've been using this for a number of days through all my troubleshooting. I know you may not want all the button changes, but the copy buttons are VERY useful to download the serial messages.