[1.1.x] report error on unsupported commands

[GMagician]

Raise an error when an unknown/unsupported G/M command is requires.

#10553 counterpart

[thinkyhead]

Sorry, this really needs to be rejected. We got complaints from host developers in the past when I took this route, and I now concur that this is a bad idea.

But let's see where they stand today…

CC: @MarlinFirmware/host-software-team

[GMagician]

Sorry, this really needs to be rejected. We got complaints from host developers in the past when I took this route, and I now concur that this is a bad idea.

this can happen when someone doesn't know the whole story...

[repetier]

In repetier-firmware we report unknown commands if error debug option is on. Not that it is the best solution when I think about it.

This is in deed a problem you can have many opinions. E.g. hosts have some commands the use for init always and since we do not know exact firmware version there might be installations not knowing the commands especially if we add new commands that did not exist in previous releases, but are useful if we get the response.

So what happens is users see an error every time they connect and then they ask host developers what is wrong with the host that it sends wrong commands. Same happens of course if slicers add unknown commands for a firmware.

Typical cases:

1. Now experts would see command xy can see yes that is not implemented, but does not matter.
2. Novice sees host does something wrong (it send a command so we are always guilty regardless who added the command :-) and they get unsure and start googling, asking questions, taking up our time.
3. Last case is slicer has wrong setting (wrong firmware settings for example) and it would show the error clearly if we check the log.

So we have cases where it would be nice to see it and cases where it is just a pain. Since we can change how old firmwares work and they will never die as "never change a running system" is consequently used by many, we can not change how it is handled backward.

One thing I would at least advice is that all new versions know all previous gcodes and would not report unknown if it is just not implemented. E.g. software power on/off might not be supported but it should not send unknown command. It should send nothing or if you persist on adding a message at least say "Not required in this version" or something like that.

In most cases it is just a command that is not implemented and all firmwares support the relevant gcodes for printing anyway. So if we issue such a message it might be nice to have additional info to say that this is normally not a problem.

To summarise: I do not really want to see them, but I see cases where it might be useful for debugging. Having an option to enable it and otherwise not see it, would be the nicest solution. So if add a M code for it (please use one the repetier does not use) we could at least switch to same solution on both firmwares.

[thinkyhead]

Here are the old issues related to this question:

First, the flames resulting from making this same change:

• Ignore G21 (set units to millimeters). #2184 : Ignore G21 (set units to millimeters)

Then, my reversion to the previous behavior to put out the fire:

• Ignore unknown commands if they start with G, M, or T #2193 : Ignore unknown commands if they start with G, M, or T.

As mentioned in #2193, there are still two debug bits free for use in M111, but a debug flag wouldn't satisfy the complaint at #10546. They needed "Unknown command: "G26"" to be reported by default, without having to send M111 S64 to the firmware (the catch-22 being it also requires reading the documentation). Even if we used the existing ERROR bit in M111, there's no guarantee that someone would be using a host that enables that bit by default. (And, it's not reported as an "error" but an "echo.")

Then, I don't relish adding responses only for certain commands — those that are never going to be used as part of a print job (e.g., G26, G33). It starts to get unwieldy to pick which codes should be given a response when disabled and which codes should not. And, surely, we don't want to only give the announcement outside of printing — when the heaters are off and the print job timer is stopped. That would be kind of strange and subvert some expectations.

I understand why host and slicer developers don't want to see "Unknown command: ..." responses, since it looks like a bug in the host or slicer, and I'm certain it would generate more support requests. And, I can understand (up to a point) why #2184 got flamed, as it was tampering with the primal forces of nature protocols. Of course it was 3 years ago when it was a PITA to slice models (often with Skeinforge!). Today it's much easier and faster, so no one worries too much about it.

So, it appears for the moment we are at an impasse.

[Zer0t3ch]

Novice sees host does something wrong (it send a command so we are always guilty regardless who added the command :-) and they get unsure and start googling, asking questions, taking up our time.

While I understand this may be confusing for some, it seems a bit backwards to prevent the firmware from keeping users informed because they might ask questions. OctoPrint is the only host software I've ever really used so I may be missing something here, but I rarely look at the terminal unless I'm running commands by hand. (in which case I think anyone would want to see errors)
Anyone developing host software can quite easily filter out the error notifications if they have a problem with their users seeing it. (either only during init where unsupported commands are common, or always; preferably as a configurable toggle for any users that care) Quite simply: I can't fathom why more verbosity is a bad thing unless it's affecting performance due to communication rate limits, especially if it can be disabled with another command.

I'm baffled that developers would want to gimp overall user-friendliness in the serial terminal because it might somehow scare others; no one should even see this output unless they're looking at terminal, anyway. Would it help if we classified these as "warnings" rather than "errors", and had the output reflect that, possibly even with an addendum to any users that they shouldn't be worried unless they know what it is?

To be clear: I don't know everything. Far from it, in fact. So I'm sorry if I'm misunderstanding something due to my lack of experience in the field.

@thinkyhead I would agree that responses for certain commands seems kinda pointless. More work than its worth and would just make things inconsistent.
As I mentioned elsewhere though, (but will repeat here for posterity) I think a generic "unknown command" would be good for any command that entirely doesn't exist, (something like G23) and then a stub function for all commands that exist but don't function for some reason (like G26 when G26_MESH_VALIDATION is undefined) that would output something like 'Feature not enabled/supported'.

[GMagician]

I can't give you an answer. For sure there are plenty of host and firmwares in the wild and different commands/commands set are implemented and is difficult to upgrade hosts everytime. Yes some of them let the user choice gcode flavour but just look at Marlin. How many new G/M commands have been implemented in the near present.
Maybe a warning is a better solution but I let expert developpers give you a better answer.

[Zer0t3ch]

@GMagician The thing that's the most confusing to me about this matter is what issues the additional output could possibly be causing. I can't imagine host softwares have programmed in every single response they expect to every single command they send wherein they arbitrarily SEGFAULT when they receive something unexpected. I find it hard to believe there's that many users freaking out about extra errors/warnings in terminal because I doubt there's that many people actually staring at the terminal anyway. This generally leads me to believe that these host softwares are just generally poor at handling unexpected responses, which should always (ironically) be expected. With one of the largest printer firmwares innovating in a relatively new field, things will be added and things will change, that's the cost of all the additional features that come with all of it.

I think as long as this kind of thing is only added into the 2.0 branch (i.e. not shipped on any devices for a while) the host software developers should have plenty of time to adapt before the masses are affected.

[thinkyhead]

I don't mean to offend anyone, but it feels as if lazy developers…

There's a contradiction in that sentence.

[Zer0t3ch]

@thinkyhead Sadly my intent diverged from my result. I've addressed it with an edit.

[ntoff]

Would the "novice users are confused" issue be cleared up if the output were more clear as to what is going on?

Rather than Recv: Unknown command: "M299" something like Recv: M299 Firmware reports command not supported or not implemented.

[thinkyhead]

As far as I'm concerned it's completely up to the host developers to decide whether they want to accept this change. Are they certain that user will flood them with bug reports, or is this just a tempest in a teapot? We need more feedback from @repetier and @foosel on their concerns and ideas.

[Roxy-3D]

There is a reasonable 'middle ground'. What if everybody agreed anything with '???' at the start of the line gets displayed to the user?

Then anything we want the user to see... But the hosts don't need... has a way to be displayed?

And if people can agree to some type of escape mechanism...

#define MSG_UNKNOWN_COMMAND "???Unknown command: ""

[repetier]

Hosts do never know all responses. In fact we never can as there are simply too many forks with local changes. So we also never expect to understand all answers and we just log them like all responses and parse what we know and need like temperatures. So adding it or not is not dependent on hosts supporting it.

@ntoff "Recv: M299 Firmware reports command not supported or not implemented" is a ver good answer from firmware if it is enabled to report unknown commands. It shows the novice "Hey, we do not implement this, but that might be ok, don't worry if everything works."

Reporting it with error flag set is not ideal as it is in deed not an error, more debug or warning message that can be ignored, but might in case of problems point to the reason. But there is no warning debug level and bits 16-64 are already used in repetier-firmware for really debugging printers (16 = Only communicate, do not execute, 32 = no moves but execute otherwise, 64 = debug endstops, report changes to log). Not sure if Marlin also has them used for other reasons. We have info but info is normally important informations and important errors often ignore error flag, e.g. decouple error which is a non maskable fatal error.

@Roxy-3D ??? not really. Involving Users to decide which escapes they want to see is too complex for normal users. What would be interesting is to have some prefix like with your echo: that indicates what kind of response it is.
echo: not really interesting, more informal or host talk. Value changes, eeprom report, ...
warning: non critical information that might be of interest.
info: Normal level of what users want to see, e.g. eeprom restored, sd print finished.
error: Something that went wrong, but print can continue
fatal: Serious error, print should stop as quickly, heaters off, ... Like you already have for decoupling reporting M999

That would go into the escape mechanism direction, but is a system already well known from logs and users can set chattiness in a combo from echo to error (fatal is always shown) with info as default.

That way we can mask host side what gets visible and even do not really require M111 to tell firmware what to hide, not hat much gets hidden anyway. M111 is really more for debugging with 1=ECHO commands and 8 = Dry run being the most important bits.

@foosel What do you think about the situation?

[ntoff]

Reporting it with error flag set is not ideal as it is in deed not an error, more debug or warning message that can be ignored,

I believe OctoPrint's default behaviour is to stop printing and totally disconnect from the printer on anything that comes back labelled "Error:" and I would tend to agree it's not erroneous that the firmware doesn't support the command, it's more of an informational response. Should this be sent back mid print with the "Error" prefix, I could see that causing quite a few headaches.

[repetier]

@ntoff I never meant to report it as error. Currently it gets only reported if error flag is set, but response is then

12:43:32.205: Unknown command: "M833"

so should be no problem with Octoprint.

[ntoff]

Sorry, I was only quoting you to agree that it's not an error. From what I gather the actual change to be implemented would report it as an error, correct? I was just agreeing with you that it's not actually an error and bringing to light OctoPrint's default behaviour as I understand it.

[Zer0t3ch]

From what I gather the actual change to be implemented would report it as an error, correct?

I think it originally was which is what spawned all the issues, but the "actual change" to be implemented in the future is part of what we're discussing here. So, whatever is does end up reporting is still up for discussion, if you have any personal suggestions as to what could be better than "Error:"

[thinkyhead]

It's not an error, so it's just prefixed with "echo:" which means: print it to inform the user. Currently we only throw "Unknown command" for commands that don't start with G,M,or T…

Unknown command: "Q123"

The first proposal we've received is to report every unrecognized command, including common commands that Marlin could handle if they were compiled in, but doesn't have compiled in. So that would include reporting very common commands… Unknown command: G29 …which just aren't compiled in.

The expanded proposal is to print a more explanatory error message like…

The firmware (on the printer) doesn't handle "G26"

Repetier and other hosts would have to always echo that message to fully satisfy the complaint, which was that the user didn't realize that there's an option, G26_MESH_EDITING that has to be enabled before G26 will do anything, and that it isn't always enabled, and that non-enabled commands are just ignored.

[Zer0t3ch]

Repetier and other hosts would have to always echo that message to fully satisfy the complaint, which was that the user didn't realize that there's an option

To clarify: do you mean that they would have to go out of their way to notify the user, (like popups of some kind) or they would just have to not hide it in terminal?

[repetier]

Without G, M or T you could even say it is a invalid command as that is the required minimum we always have. That is really uncritical to report.

[thinkyhead]

they would just have to not hide it in terminal

This. And, you know, make a loud noise so the user turns their head and actually reads the log.

[Zer0t3ch]

Are you being sarcastic? Or do you really think users should be drawn to this information?

My assumption was that this kind of information would only really matter to people entering gcode by hand, where they would naturally see the output. That said, I'm not sure where my use-case differs from others'.

[thinkyhead]

do you really think users should be drawn to this information?

I think modeless operation is best for a host. It might at least help to print the message in red letters. It's all too easy to make a typo from the host command line. Modal alerts should be reserved for more serious conditions.

[foosel]

Something like

Error: The firmware doesn't handle "G29"

would be bad. The wiki (which is the closest thing that we have with regards to a specification, though to be taken with a grain of salt since everyone and their mother can edit it) defines the error prefix (or rather the !! prefix which is equivalent) as "means that a hardware fault has been detected. The RepRap machine will shut down immediately after it has sent this message." This is also why OctoPrint's default behaviour is to throw up its hands in frustration and disconnect when it sees an error that isn't caused by a communication failure (and known to be followed by a resend request) or known to be non fatal ("can't mount SD card"). I could add another exception for an unknown command error, but I would really appreciate it if this could instead be prefixed with "Warn:" or just "echo:" instead of keeping mixing non fatal errors in with actual fatal ones.

Error: Unknown command: G29 would in theory also be bad, but due to some firmware variant out there already using this approach OctoPrint actually does have an exception for this in place and has had this whitelisted as non fatal (rather, any "Error:" that contains "unknown command" in its lower case) since 1.2.8 (late 2015).

As long as you don't prefix non-fatal messages with Error: or !! I'm fairly relaxed. As @repetier already said, host software can't parse each and every message from firmware anyhow, we just ignore stuff we don't know/don't specifically handle. Too many forks out there that are largely incompatible to each other to even attempt to do that.

Now, about that prefix idea, I agree that that would be nice, but if you decide to go down this route (which I would say is probably a different discussion than this one), please make sure to involve the host devs ;) What definitely cannot be done here is redefine what "Error:" means (fatal vs "meh, can continue") - I'd see huge backwards compatibility issues here, especially during initial connection to a printer way before I have any way to know or chance to figure out what that specific firmware variant might mean with an "Error:".

Also keep in mind with this change that sometimes during initial connect a command might not make it over fully or be otherwise corrupted. I've seen printers respond with an "Unknown command" as response to an M110 right on connect which accepted this perfectly fine a couple of seconds later.

[thinkyhead]

@foosel — Currently (with the recent change) all unknown G-codes will send a string like this:

echo: Unknown command: "Q123 R45 S67"

AFAICT M110 is always handled, so it shouldn't come back unknown. Maybe a glitch in an old Marlin?

[foosel]

all unknown G-codes will send a string like this:

That's fine and shouldn't cause any technical problem. I can imagine some users will blame this on the host again (just as I constantly get tickets opened for MINTEMP errors 😒) but from my experience even if you report something like this to the user with "Your printer's firmware reported the following error" a ton of users out there don't understand where the issue is coming from, so all the wording changes in the world won't be able to make this crystal clear for everyone ¯\_(ツ)_/¯

Maybe a glitch in an old Marlin?

More than likely. Just wanted to raise this point since I observed it in the field in some provided serial.logs.

[dot-bob]

G-code by is really machine to machine communications. Perhaps error messages could be classified in the response to help reduce confusion and help the host parse the messages. For instance use a response like this...
ERR:01: "Unknown command Q123 R45 S67"
Where ERR:01 can be parsed by the host software and the data "Unknown command Q123 R45 S67" can optionally be passed to the user.
Syntax then would look like the following:
ERR:XX: "Message"
Where XX is
01 - Undefined Command
02 - Invalid argument
03 - Critical Error
04 - Warning
05 - Debugging

[Zer0t3ch]

G-code by is really machine to machine communications

I absolutely gotta disagree with this in general.

That said, I have nothing against your suggestion for error response, as it's still relatively easy for a human to comprehend. (albeit very brief) Though I think the majority of what you're describing is already kind of supported with debug flags, so it may be unnecessary to implement so much. And it's also good to keep in mind what some others have said already: we should probably avoid the term "error" all-together as it implies something fatal already.

[dot-bob]

The example I provided above was how I have seen this problem solved in the past. The specific syntax was from a satellite transmitter I had used in the past. The message doesn't need to include ERR it could be :02: or something else. The idea is to prefix or post fix the message with a language independent code that the host can interpret and decide if it needs to perform an action. The message string can be passed through to the UI of the host if desired or just used for low level debugging by the user.

It would need to be incorporated in such a way so it doesn't break current compatibility just extends capability. It would probably need to be implemented as a mode that can be enabled.

The big question is does it give enough benefit this late in the game to implement something like this.

[Zer0t3ch]

Okay, I get what you're saying. Maybe combine some of the ideas here, something like ???:001:Unsupported command?

[repetier]

Do not really like ERR:01 solution. That is too specific for a solution that needs to cover 100s of firmware forks. There is no need to start with such cryptic versions where everyone starts adding new numbers to ERR which is no error at all.

Best solution as it looks is to leave it as echo, just make sure users can see that this is no big problem, like
echo: The firmware (on the printer) doesn't handle "G26"

Hosts will just show it and do not freak out like if error word is involved, it explains it to novice that it is just not handled and experts still see that a command was ignored.

The only real alternative would be to have a general log level like response so hosts could filter low level reports in a range of
debug:
echo:
warning:
info:
error:

in such a system it could get warning level. These levels are common and easy to copy on other firmwares if they want to support it. In that case firmware should add a capability flag that such a log level system is provided, so they can offer filters with "info" being the default visible to users.