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

Add absolute speed option for Smoothieboard driver #63

Closed
wants to merge 1 commit into from

Conversation

pelrun
Copy link
Contributor

@pelrun pelrun commented Sep 23, 2016

Adds new Smoothie driver that changes the meaning of the Speed variable from percent to mm/min.

Needs more work, don't merge just yet.

t-oster/VisiCut#381

@pelrun pelrun force-pushed the absolutespeed branch 2 times, most recently from 794a9b5 to 233ba3f Compare September 25, 2016 08:30
@mgmax
Copy link
Collaborator

mgmax commented Sep 25, 2016

As this is also applicable to all other GenericGcode based drivers, it would perhaps be better to make this a configuration option inside GenericGcodeDriver, named something like "Absolute speed (change will break existing settings)". It would also make the code easier to understand, if setSpeed just contains an if/else logic.

The configuration dialog even allows changing the driver, so we cannot prevent the user from switching the option anyway.

The Epilog driver is not based on Gcode, so it would still work as before.

@pelrun
Copy link
Contributor Author

pelrun commented Sep 25, 2016

I don't like having an option break all the profiles, especially when you can't then tell which ones have been updated. But I'm not happy with having multiple drivers either. The option needs to either auto-update the profiles, or just change how they're displayed. I'll look at doing the latter.

Also, I made the driver immutable a while back, because you couldn't safely change it then change it back without losing some of the settings. And it happened immediately, so hitting "Cancel" didn't fix it.

@madleech
Copy link
Contributor

Cool feature, I'd be very interested in having this as a general option for all gcode drivers.

@t-oster
Copy link
Owner

t-oster commented Sep 26, 2016

I am not sure, if it is such a problem that it breaks the profiles, because if you are messing with the driver settings, you should know what you are doing anyway...
....@madleech nice avatar....

@pelrun
Copy link
Contributor Author

pelrun commented Sep 26, 2016

It's still a UI failure. And fixing those has been the main focus of my work on VisiCut.

@t-oster
Copy link
Owner

t-oster commented Dec 7, 2016

Hi, what is the state of this patch? I think adding it to the generic gcode driver is a good option

@pelrun
Copy link
Contributor Author

pelrun commented Dec 7, 2016

I'm not happy with it, and I didn't get far trying to resolve the issues that popped up. The big one was that the displayed text for the "power/speed/focus" options is magic and used as a key for serialisation. So I couldn't readily just change those dynamically to include units, because it breaks everything.

It did occur to me you can be super sneaky and just change the max speed in the driver to 6000 mm/min, at which point the percentage is also numerically identical to the speed in mm/s...

@t-oster
Copy link
Owner

t-oster commented Dec 7, 2016

but it will only allow values between 0 and 100, right?

@pelrun
Copy link
Contributor Author

pelrun commented Dec 7, 2016

It's just a trick, not really a good solution :)

@pelrun
Copy link
Contributor Author

pelrun commented Jan 9, 2017

I'm having another stab at this, although the implementation is significantly different now - so I'll close this PR and raise a new one if/when I get it working.

@pelrun pelrun closed this Jan 9, 2017
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

Successfully merging this pull request may close these issues.

None yet

4 participants