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

M408 #6970

Closed
wants to merge 5 commits into from
Closed

M408 #6970

wants to merge 5 commits into from

Conversation

akaJes
Copy link
Contributor

@akaJes akaJes commented Jun 6, 2017

@bgort
Copy link
Contributor

bgort commented Jun 6, 2017

Out of curiosity, what's the (or your) use case for this?

.gitignore Outdated
@@ -136,3 +136,7 @@ CMakeListsPrivate.txt

#CLion
cmake-build-*
.pioenvs
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This and the following one are already covered by line 120.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

these were auto appended with gui

@akaJes
Copy link
Contributor Author

akaJes commented Jun 7, 2017

@bgort I would like to create a web interface
@thinkyhead have you some seconds to look at this PR ?

@bgort
Copy link
Contributor

bgort commented Jun 9, 2017

I don't see any reason not to merge this. It's conditionally compiled, and as long as it works, I can't see how it hurts anything. Let's give @thinkyhead a bit longer to comment and then I'll merge it.

.travis.yml Outdated
- restore_configs
- opt_enable REPORT_M408_JSON
- build_marlin
#
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Enable the option in one of the other tests, rather than requiring an additional build.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

which is more prefered? PRINTCOUNTER ?

SERIAL_PROTOCOLPGM("],\"fraction_printed\": ");
SERIAL_PROTOCOL_F(card.percentDone() * 0.01, 3);
#else
SERIAL_PROTOCOLPGM("]");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

SERIAL_PROTOCOLCHAR(']');

}
#endif
//skipped standbay & hstat
SERIAL_PROTOCOLPGM("],\"pos\": [");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These custom print statements that create key/value strings should probably be implemented as a small function taking the key and value. The code will likely end up smaller.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thought about this, but don't see good solution yet

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A single key-value output would be…

void floatkey(const char *pstr, const float &val, const bool comma=false) {
  SERIAL_CHAR('"'); serialprintPGM(pstr); SERIAL_CHAR('"');
  SERIAL_CHAR(':'); SERIAL_ECHO(val);
  if (comma) SERIAL_ECHO(',');
}

And a function for XYZ elements…

void xyzkey(const char *pstr, const float val[XYZ], const bool comma=false) {
  SERIAL_CHAR('"'); serialprintPGM(pstr); SERIAL_CHAR('"');
  SERIAL_ECHO(":[");
  SERIAL_ECHO(val[X_AXIS]); SERIAL_CHAR(',');
  SERIAL_ECHO(val[Y_AXIS]); SERIAL_CHAR(',');
  SERIAL_ECHO(val[Z_AXIS]);
  SERIAL_CHAR(']');
  if (comma) SERIAL_ECHO(',');
}

if (print_job_timer.isPaused())
SERIAL_PROTOCOLCHAR('A'); // PAUSED
else
SERIAL_PROTOCOLCHAR('B'); // SOMETHING ELSE
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please put else if keywords together on the same line. Or use this:

SERIAL_PROTOCOLCHAR(
  !planner.blocks_queued()                        ? 'I' : // IDLING
  (IS_SD_PRINTING || print_job_timer.isRunning()) ? 'P' : // SD Printing
  print_job_timer.isPaused()                      ? 'A' : // PAUSED
                                                    'B'   // SOMETHING ELSE
);

#else
SERIAL_PROTOCOLPGM("]");
#endif
SERIAL_PROTOCOLLNPGM("}");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

SERIAL_PROTOCOLCHAR('}');

@akaJes akaJes changed the base branch from bugfix-1.1.x to bugfix-2.0.x July 31, 2017 10:25
@thinkyhead
Copy link
Member

thinkyhead commented Nov 14, 2017

Something screwy is going on here. The base was changed to bugfix-2.0.x on July 31, but the PR has changes to Marlin_main.cpp, which is only in the 1.1 layout.

Please redo.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants