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

Allow stopwatch and printcounter to go over 18:12:15 #4287

Merged
merged 6 commits into from
Jul 14, 2016

Conversation

thinkyhead
Copy link
Member

@thinkyhead thinkyhead commented Jul 13, 2016

Addressing #4195

After 18 hours, 12 minutes, and 16 seconds, a 16-bit integer counting seconds will overflow. This PR alters variables in stopwatch and printcounter to use millis_t instead of 16-bit integers.

  • Use wider integers to store time durations
  • Extend M31 to include days in its output
  • Run M31 at the end of an SD print rather than custom-printing the time

@thinkyhead thinkyhead mentioned this pull request Jul 13, 2016
@thinkyhead thinkyhead force-pushed the rc_long_print_times branch 2 times, most recently from 259eb10 to 54e955b Compare July 13, 2016 02:35
@thinkyhead
Copy link
Member Author

thinkyhead commented Jul 13, 2016

@MarlinFirmware/host-software-team
Do hosts expect the output of M31 to never include hours (only min and sec)?

@foosel
Copy link
Contributor

foosel commented Jul 13, 2016

Stock OctoPrint doesn't even parse M31 output, so wouldn't be affected by any changes there. Don't know about other hosts though.

@petrzjunior
Copy link
Contributor

@thinkyhead Oh, good idea to prevent overflow! I was trying to do exactly the same yesterday, but I wasn't successful enough to push it.

I'm not sure, what millis_t is, but I was thinking about uint32_t. This would extend it to 4 billion second, roughly 50 000 days. I was also trying to reformat the info menu to show the timer on new line as "_days_d _hours_h _minutes_m", this would with 50 thousand days fit perfectly even into 14 chars on LCD.

But strange things happened and this snippet

sprintf(printTime, "%02ud %02uh %02um", 
        (uint16_t)(stats.printTime / 86400), 
        ((uint32_t)(stats.printTime / 60)) % 60, 
        ((uint32_t)(stats.printTime / 3600)) % 24
);

doesn't work on Arduino while in online C++ Shell does.
It skips the middle (sometimes the last) value while formatting. Strange. Blindness or Arduino bug?

I would certainly implement days, since @clexpert's printer is printing more than someone would expect. 😄

@thinkyhead
Copy link
Member Author

thinkyhead commented Jul 13, 2016

I'm not sure, what millis_t is…

A search of the source code reveals…

typedef unsigned long millis_t;

strange things happened and this snippet

I would suggest instead…

sprintf(printTime, "%id %02ih %02im", 
        int(stats.printTime / 60 / 60 / 24),
        int(stats.printTime / 60 / 60) % 60,
        int(stats.printTime / 60) % 60
);

@thinkyhead thinkyhead force-pushed the rc_long_print_times branch 2 times, most recently from fcbdb68 to f48dba3 Compare July 13, 2016 20:11
@thinkyhead thinkyhead force-pushed the rc_long_print_times branch 4 times, most recently from 5e3afc6 to febf394 Compare July 14, 2016 18:20
@thinkyhead thinkyhead force-pushed the rc_long_print_times branch from febf394 to 1618870 Compare July 14, 2016 18:40
@thinkyhead thinkyhead merged commit 75901b6 into MarlinFirmware:RCBugFix Jul 14, 2016
@thinkyhead thinkyhead deleted the rc_long_print_times branch July 14, 2016 19:14
@thinkyhead thinkyhead added this to the 1.1.0 milestone Jul 14, 2016
#define MSG_INFO_PRINT_TIME "Gesamte Druckzeit"

#if LCD_WIDTH > 19
#define MSG_INFO_TOTAL_PRINTS "Gesamte Drucke "
Copy link

Choose a reason for hiding this comment

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

It seems that MSG_INFO_PRINT_COUNT is right.

@ghost
Copy link

ghost commented Jul 15, 2016

Excuse me I've a question, are MSG_END_DAY, MSG_END_HOUR, and MSG_END_MINUTE used where?

@jbrazio
Copy link
Contributor

jbrazio commented Jul 15, 2016

I can't find their usage anywhere in the code (current RCBugfix).. maybe on a unmerged PR ?

@Blue-Marlin
Copy link
Contributor

Yesterday removed with 34da77d

@thinkyhead
Copy link
Member Author

Hmm, guess those strings are now obsolete. Print time on the display is now coming from M31.

@ghost
Copy link

ghost commented Jul 16, 2016

I see, thus I'll remove those from language files that I'm managing.

thinkyhead added a commit that referenced this pull request Jul 17, 2016
Follow-up and fix the PR #4287 (Allow stopwatch and printcounter to go over 18:12:15)
@kakaroto
Copy link

a bit late here, but just to confirm that Cura LE also doesn't use M31 and counts the print time internally (and stops/resumes the counter when user pauses/resumes the print).

CONSULitAS pushed a commit to CONSULitAS/Marlin-K8200 that referenced this pull request Aug 18, 2016
…ntcounter to go over 18:12:15)

・Remove MSG_END_HOUR and MSG_END_MINUTE from all the language files
・Change from MSG_INFO_TOTAL_PRINTS to MSG_INFO_PRINT_COUNT in German
file
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.

6 participants