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

[Request] Automatically uppercase Gcode parameters where it makes sense #1026

Closed
eovnu87435ds opened this issue Aug 17, 2015 · 12 comments
Closed
Labels
done Done but not yet released request Feature request

Comments

@eovnu87435ds
Copy link

I'm not sure if this is an issue or not, but I found that if you type a gcode command with no arguments into the terminal page, it works whether the letter is upper or lower-case(g28 and G28 both work). Commands with multiple arguments only work if each letter is uppercase. (M280 P0 S150 works, m280 p0 s150 does not).

I noticed that if I use a different host program, such as Repetier host, upper and lower case both work.

1. What were you doing? Sending mixed-case gcode through the terminal page

2. What did you expect to happen? The printer would execute the gcode

3. What happened instead? The printer did not execute gcode which had more than 0 arguments and was mixed case

4. Branch & Commit or Version of OctoPrint: 1.2.4 (master branch) via OctoPi

5. Printer model & used firmware incl. version
(if applicable - always include if unsure):
Folger Technologies RepRap Kossel running Marlin firmware

6. Browser and Version of Browser, Operating
System running Browser (if applicable - always
include if unsure):
Windows 8.1x64 using Chrome 44.0.2403.155 m

7. Link to octoprint.log on gist.github.com or pastebin.com
(ALWAYS INCLUDE AND DO NOT TRUNCATE):
see http://pastebin.com/8xhwukWV

8. Link to contents of terminal tab or serial.log on
gist.github.com or pastebin.com (if applicable - always
include if unsure or reporting communication issues AND
DO NOT TRUNCATE):
See terminal tab, specifically lines 36-58 http://pastebin.com/KKS8xkd7

9. Link to contents of Javascript console in the browser
on gist.github.com or pastebin.com or alternatively a
screenshot (if applicable - always include if unsure
or reporting UI issues):

10. Screenshot(s) showing the problem (if applicable - always
include if unsure or reporting UI issues):

I have read the FAQ.

@foosel
Copy link
Member

foosel commented Aug 18, 2015

First of all: Thank you for the effort put into this ticket. Not many people read the ticket guidelines, follow them and then even make sure that the ticket is formatted well. That makes looking into this so much easier.

Now, regarding your problem, the current behaviour is intentional due to reasons I'm going to outline in a second, but I'm open to discuss a better solution.

The thing is that I can't just blindly upper-case everything that the user inputs into that box. The M117 command for example can be used to send arbitrary text to be displayed on the printer's LCD, and that certainly should not be upper-cased. A blacklist ("Commands not to uppercase") or a whitelist ("Commands to uppercase") approach are tricky here, since available commands can change a lot (e.g. in experimental firmware builds or simply vendor specific firmware builds).

I guess an "uppercase" blacklist, off the shelve only prefilled with M117 might actually work though. Thoughts?

@nophead
Copy link
Contributor

nophead commented Aug 18, 2015

G code is case insensitive, so it is a firmware bug. Why paper over in the
host?

On 18 August 2015 at 20:01, Gina Häußge notifications@github.com wrote:

First of all: Thank you for the effort put into this ticket. Not many
people read the ticket guidelines, follow them and then even make sure that
the ticket is formatted well. That makes looking into this so much easier.

Now, regarding your problem, the current behaviour is intentional due to
reasons I'm going to outline in a second, but I'm open to discuss a better
solution.

The thing is that I can't just blindly upper-case everything that the user
inputs into that box. The M117 command for example can be used to send
arbitrary text to be displayed on the printer's LCD, and that certainly
should not be upper-cased. A blacklist ("Commands not to uppercase") or a
whitelist ("Commands to uppercase") approach are tricky here, since
available commands can change a lot (e.g. in experimental firmware builds
or simply vendor specific firmware builds).

I guess an "uppercase" blacklist, off the shelve only prefilled with M117
might actually work though. Thoughts?


Reply to this email directly or view it on GitHub
#1026 (comment).

@foosel
Copy link
Member

foosel commented Aug 18, 2015

Why paper over in the host?

Because I can quite easily make life for users easier here by doing that without making OctoPrint harder to maintain. I can't however change the quadrillion separate versions of firmwares that are out there. Heck, my actual fixes of definite Marlin bugs I can't workaround in OctoPrint are still not even remotely close to being propagated across the hundreds of forks of that firmware out there and keep popping up again and again on the mailing list and in this very bug tracker here.

G code is case insensitive

The NIST standard says yes (quote, "Input is case insensitive, except in comments, i.e., any letter outside a comment may be in upper or lower case without changing the meaning of a line."), the RepRap wiki suggests otherwise (examples are uppercase only across the board). As usual, people will tell you that only the document is valid that best fits their own implementation ;)

@ntoff
Copy link
Contributor

ntoff commented Aug 22, 2015

Because I can quite easily make life for users easier here by doing that without making OctoPrint harder to maintain. I can't however change the quadrillion separate versions of firmwares that are out there. Heck, my actual fixes of definite Marlin bugs I can't workaround in OctoPrint are still not even remotely close to being propagated across the hundreds of forks of that firmware out there and keep popping up again and again on the mailing list and in this very bug tracker here.

Why worry about the other firmwares? It seems like the main Marlin devs and possibly the repetier devs are willing to work with you to fix bugs and change things, let the people who forked the firmware worry about implementing those fixes and if they don't bother implementing it then that's their problem and not yours.

How hard would it be to make it uppercase everything NOT inside of quote marks? So you could send m280 p0 s150 (example from the OP) and have it auto uppercase it, or send m117 "This is a non auto case" and it'd only uppercase the M117 bit. Is that a dumb idea? Too hard? I have no idea :(

@foosel
Copy link
Member

foosel commented Aug 22, 2015

let the people who forked the firmware worry about implementing those fixes

I don't believe in screwing over the users because we as a 3d printing community can't get our infrastructure together (e.g. a common standardized well-defined protocol and non-scary upgrade paths for people to get their printers flashed with fixed firmware versions).

How hard would it be to make it uppercase everything NOT inside of quote marks

Not hard at all, except: M117 doesn't necessitate quotation marks ;) In fact, I think if you send them along they'll be displayed. So - not a dumb idea, but not applicable to the problem.

@foosel foosel changed the title Gcode only case-insensitive for 0 argument commands [Request] Automatically uppercase Gcode parameters where it makes sense Feb 15, 2016
@foosel foosel added the request Feature request label Feb 15, 2016
@foosel foosel added up-for-grabs good first issue A good first issue for someone new to OctoPrint's development labels Feb 24, 2016
@drogge
Copy link

drogge commented Mar 21, 2016

What about adding a checkbox to the bottom of the Terminal tab to enable/disable uppercasing the text in the box? That way the user can deal with the special commands that shouldn't be uppercased.

Also, some of the SD m codes take pretty much free form input as an argument.

@agarwali
Copy link
Contributor

@foosel My undergrad team and I would like to tackle this issue. We could parse the G-Code command input string to make everything uppercase but M117. What do you think would be a good approach?

@foosel
Copy link
Member

foosel commented Mar 29, 2016

@agarwali Sounds good! I'm unsure about limiting it to M117 only though and would rather suggest matching the detected GCODE against a (configurable) list of GCODEs for which to not do the uppercasing, defaulting to only one member M117.

It might be necessary to also take a hard look at the SD codes @drogge mentioned, like M30 etc. As far as I remember the file names are case insensitive anyhow (visible also by M20 always returning uppercase names but there not being any problems with selecting and printing files through lower case names either), but testing that against a real printer with the filter in place and verifying that nothing breaks would be good, or even adding the file handling GCODEs to the list as well (full list of commands that I'm aware of here: M23, M28, M29, M30, M32, M33)

So, in a nutshell:

  • add new config setting: blacklist of commands to not completely auto-uppercase (incl. setting dialog, I think it would probably fit into the "Advanced Options" section on the Serial tab, since it's serial related?)
  • adjust sendCommand method of TerminalViewModel: extract GCODE, match against blacklist. For matches only uppercase GCODE, for non matches uppercase full command before sending

@foosel foosel added grabbed Grabbed by someone from the community to be implemented/fixed and removed up-for-grabs labels Mar 29, 2016
@drogge
Copy link

drogge commented Mar 29, 2016

That sounds great! A configurable list with an enable checkbox should cover all cases. Would be an interesting project that would involve both UI work and the actual implementation of the case shifting. I'm not sure if there's a case where part of a gcode command would want to be uppercased and another part wouldn't. I can imagine something like the M117 that would take a parameter for the amount of time to display the message but I don't think anything like that exists.

agarwali pushed a commit to agarwali/OctoPrint that referenced this issue Apr 30, 2016
Added new config setting: auto uppercase blacklist that is configurable in the advanced option of the serial tab, with M117 included in the default settings of the blacklist.
Changed the sendCommand method in the TerminalViewModel to auto uppercase the entire gcode command except when gcode is in the blacklist.
Updated documentaion to include the defualt auto uppercase blacklist command M117.

Resolves: OctoPrint#1026
agarwali added a commit to agarwali/OctoPrint that referenced this issue Apr 30, 2016
Added new config setting: auto uppercase blacklist that is configurable in the advanced option of the serial tab, with M117 included in the default settings of the blacklist.
Changed the sendCommand method in the TerminalViewModel to auto uppercase the entire gcode command except when gcode is in the blacklist.
Updated documentaion to include the defualt auto uppercase blacklist command M117.

Resolves: OctoPrint#1026
agarwali added a commit to agarwali/OctoPrint that referenced this issue Apr 30, 2016
Added new config setting: auto uppercase blacklist that is configurable in the advanced option of the serial tab, with M117 included in the default settings of the blacklist.
Changed the sendCommand method in the TerminalViewModel to auto uppercase the entire gcode command except when gcode is in the blacklist.
Updated documentaion to include the defualt auto uppercase blacklist command M117.

Resolves: OctoPrint#1026
agarwali added a commit to agarwali/OctoPrint that referenced this issue Apr 30, 2016
Added new config setting: auto uppercase blacklist that is configurable in the advanced option of the serial tab, with M117 included in the default settings of the blacklist.
Changed the sendCommand method in the TerminalViewModel to auto uppercase the entire gcode command except when gcode is in the blacklist.
Updated documentaion to include the defualt auto uppercase blacklist command M117.

Resolves: OctoPrint#1026
@jandatax
Copy link

Is there any progress on this?
This would make life a bit easier for me.

If we cannot expect this feature in the near future, could you let me know.
I could modify my own code to simply uppercase everything, but was holding off on that waiting for the fix.

@foosel
Copy link
Member

foosel commented Jul 27, 2017

The PR #1321 sadly never was updated after review and I so far didn't have time to tackle this myself. If anyone would adopt the PR and give it the final polish that would be great. Otherwise this will take longer.

@foosel foosel added up-for-grabs and removed grabbed Grabbed by someone from the community to be implemented/fixed labels Jul 27, 2017
pbackx added a commit to pbackx/OctoPrint that referenced this issue Oct 25, 2017
foosel pushed a commit that referenced this issue Nov 3, 2017
…lly work [#1026]

(cherry picked from commit 1857f07)
@foosel
Copy link
Member

foosel commented Nov 3, 2017

Huge thanks to @pbackx for picking up the reigns and giving the aforementioned PR the final polish in #2177. This feature has now been merged to maintenance, soon devel and will be part of the 1.3.6 release.

@foosel foosel added the done Done but not yet released label Nov 3, 2017
@foosel foosel removed good first issue A good first issue for someone new to OctoPrint's development up-for-grabs labels Nov 3, 2017
@foosel foosel added this to the 1.3.6 milestone Nov 3, 2017
@foosel foosel closed this as completed in 51406b7 Dec 12, 2017
@github-actions github-actions bot locked as resolved and limited conversation to collaborators May 29, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
done Done but not yet released request Feature request
Projects
None yet
Development

No branches or pull requests

7 participants