Skip to content

Shared AutoReport class for Temperature, Cardreader #20913

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.

Sign up for GitHub

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

Merged
thinkyhead merged 2 commits into from
Jan 29, 2021
Merged

Shared AutoReport class for Temperature, Cardreader #20913

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
a66efa5
Apply SEC_TO_MS macro
thinkyhead Jan 29, 2021
3909cea
Shared AutoReport class for Temperature, Cardreader
thinkyhead Jan 29, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions Marlin/src/MarlinCore.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -687,8 +687,8 @@ void idle(TERN_(ADVANCED_PAUSE_FEATURE, bool no_stepper_sleep/*=false*/)) {
// Auto-report Temperatures / SD Status
#if HAS_AUTO_REPORTING
if (!gcode.autoreport_paused) {
TERN_(AUTO_REPORT_TEMPERATURES, thermalManager.auto_report_temperatures());
TERN_(AUTO_REPORT_SD_STATUS, card.auto_report_sd_status());
TERN_(AUTO_REPORT_TEMPERATURES, thermalManager.auto_reporter.tick());
TERN_(AUTO_REPORT_SD_STATUS, card.auto_reporter.tick());
}
#endif

Expand Down
2 changes: 1 addition & 1 deletion Marlin/src/gcode/calibrate/G76_M192_M871.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -267,7 +267,7 @@ void GcodeSuite::G76() {

say_waiting_for_probe_heating();
SERIAL_ECHOLNPAIR(" Bed:", target_bed, " Probe:", target_probe);
const millis_t probe_timeout_ms = millis() + 900UL * 1000UL;
const millis_t probe_timeout_ms = millis() + SEC_TO_MS(900UL);
while (thermalManager.degProbe() < target_probe) {
if (report_temps(next_temp_report, probe_timeout_ms)) {
SERIAL_ECHOLNPGM("!Probe heating timed out.");
Expand Down
2 changes: 1 addition & 1 deletion Marlin/src/gcode/sd/M1001.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ void GcodeSuite::M1001() {
printerEventLEDs.onPrintCompleted();
TERN_(EXTENSIBLE_UI, ExtUI::onUserConfirmRequired_P(GET_TEXT(MSG_PRINT_DONE)));
TERN_(HOST_PROMPT_SUPPORT, host_prompt_do(PROMPT_USER_CONTINUE, GET_TEXT(MSG_PRINT_DONE), CONTINUE_STR));
wait_for_user_response(1000UL * TERN(HAS_LCD_MENU, PE_LEDS_COMPLETED_TIME, 30));
wait_for_user_response(SEC_TO_MS(TERN(HAS_LCD_MENU, PE_LEDS_COMPLETED_TIME, 30)));
printerEventLEDs.onResumeAfterWait();
}
#endif
Expand Down
10 changes: 6 additions & 4 deletions Marlin/src/gcode/sd/M27.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,15 +36,17 @@ void GcodeSuite::M27() {
if (parser.seen('C')) {
SERIAL_ECHOPGM("Current file: ");
card.printFilename();
return;
}

#if ENABLED(AUTO_REPORT_SD_STATUS)
else if (parser.seenval('S'))
card.set_auto_report_interval(parser.value_byte());
if (parser.seenval('S')) {
card.auto_reporter.set_interval(parser.value_byte());
return;
}
#endif

else
card.report_status();
card.report_status();
}

#endif // SDSUPPORT
2 changes: 1 addition & 1 deletion Marlin/src/gcode/temp/M155.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@
void GcodeSuite::M155() {

if (parser.seenval('S'))
thermalManager.set_auto_report_interval(parser.value_byte());
thermalManager.auto_reporter.set_interval(parser.value_byte());

}

Expand Down
2 changes: 1 addition & 1 deletion Marlin/src/lcd/dwin/e3v2/dwin.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3553,7 +3553,7 @@ void EachMomentUpdate() {
static millis_t next_remain_time_update = 0;
if (Percentrecord > 1 && ELAPSED(ms, next_remain_time_update) && !HMI_flag.heat_flag) {
remain_time = (elapsed.value - dwin_heat_time) / (Percentrecord * 0.01f) - (elapsed.value - dwin_heat_time);
next_remain_time_update += 20 * 1000UL;
next_remain_time_update += SEC_TO_MS(20);
Draw_Print_ProgressRemain();
}
}
Expand Down
4 changes: 2 additions & 2 deletions Marlin/src/lcd/extui/lib/ftdi_eve_touch_ui/compat.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,8 +43,8 @@
#define min(a,b) ((a)<(b)?(a):(b))
#else
namespace UI {
static inline uint32_t safe_millis() {return millis();};
static inline void yield() {};
static inline uint32_t safe_millis() { return millis(); }
static inline void yield() {}
};
#endif

Expand Down
6 changes: 3 additions & 3 deletions Marlin/src/lcd/extui/lib/mks_ui/draw_ui.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -889,7 +889,7 @@ void GUI_RefreshPage() {
lv_draw_wifi_tips();

}
if (tips_disp.timer_count >= 30 * 1000) {
if (tips_disp.timer_count >= SEC_TO_MS(30)) {
tips_disp.timer = TIPS_TIMER_STOP;
tips_disp.timer_count = 0;
lv_clear_wifi_tips();
Expand All @@ -898,7 +898,7 @@ void GUI_RefreshPage() {
}
break;
case TIPS_TYPE_TAILED_JOIN:
if (tips_disp.timer_count >= 3 * 1000) {
if (tips_disp.timer_count >= SEC_TO_MS(3)) {
tips_disp.timer = TIPS_TIMER_STOP;
tips_disp.timer_count = 0;

Expand All @@ -908,7 +908,7 @@ void GUI_RefreshPage() {
}
break;
case TIPS_TYPE_WIFI_CONECTED:
if (tips_disp.timer_count >= 3 * 1000) {
if (tips_disp.timer_count >= SEC_TO_MS(3)) {
tips_disp.timer = TIPS_TIMER_STOP;
tips_disp.timer_count = 0;

Expand Down
8 changes: 4 additions & 4 deletions Marlin/src/lcd/extui/lib/mks_ui/tft_lvgl_configuration.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -89,17 +89,17 @@ void SysTick_Callback() {
#endif
if (uiCfg.filament_loading_time_flg) {
uiCfg.filament_loading_time_cnt++;
uiCfg.filament_rate = (uint32_t)(((uiCfg.filament_loading_time_cnt / (uiCfg.filament_loading_time * 1000.0)) * 100.0) + 0.5);
if (uiCfg.filament_loading_time_cnt >= (uiCfg.filament_loading_time * 1000)) {
uiCfg.filament_rate = uint32_t(100.0f * uiCfg.filament_loading_time_cnt / SEC_TO_MS(uiCfg.filament_loading_time) + 0.5f);
if (uiCfg.filament_loading_time_cnt >= SEC_TO_MS(uiCfg.filament_loading_time)) {
uiCfg.filament_loading_time_cnt = 0;
uiCfg.filament_loading_time_flg = false;
uiCfg.filament_loading_completed = true;
}
}
if (uiCfg.filament_unloading_time_flg) {
uiCfg.filament_unloading_time_cnt++;
uiCfg.filament_rate = (uint32_t)(((uiCfg.filament_unloading_time_cnt / (uiCfg.filament_unloading_time * 1000.0)) * 100.0) + 0.5);
if (uiCfg.filament_unloading_time_cnt >= (uiCfg.filament_unloading_time * 1000)) {
uiCfg.filament_rate = uint32_t(100.0f * uiCfg.filament_unloading_time_cnt / SEC_TO_MS(uiCfg.filament_unloading_time) + 0.5f);
if (uiCfg.filament_unloading_time_cnt >= SEC_TO_MS(uiCfg.filament_unloading_time)) {
uiCfg.filament_unloading_time_cnt = 0;
uiCfg.filament_unloading_time_flg = false;
uiCfg.filament_unloading_completed = true;
Expand Down
6 changes: 3 additions & 3 deletions Marlin/src/lcd/extui/ui_api.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,7 @@ namespace ExtUI {
// Machine was killed, reinit SysTick so we are able to compute time without ISRs
if (currTimeHI == 0) {
// Get the last time the Arduino time computed (from CMSIS) and convert it to SysTick
currTimeHI = (uint32_t)((GetTickCount() * (uint64_t)(F_CPU / 8000)) >> 24);
currTimeHI = uint32_t((GetTickCount() * uint64_t(F_CPU / 8000)) >> 24);

// Reinit the SysTick timer to maximize its period
SysTick->LOAD = SysTick_LOAD_RELOAD_Msk; // get the full range for the systick timer
Expand All @@ -148,9 +148,9 @@ namespace ExtUI {
}
#endif // __SAM3X8E__

void delay_us(unsigned long us) { DELAY_US(us); }
void delay_us(uint32_t us) { DELAY_US(us); }

void delay_ms(unsigned long ms) {
void delay_ms(uint32_t ms) {
if (flags.printer_killed)
DELAY_US(ms * 1000);
else
Expand Down
6 changes: 3 additions & 3 deletions Marlin/src/lcd/extui/ui_api.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -155,7 +155,7 @@ namespace ExtUI {
void onMeshUpdate(const int8_t xpos, const int8_t ypos, const float zval);
inline void onMeshUpdate(const xy_int8_t &pos, const float zval) { onMeshUpdate(pos.x, pos.y, zval); }

typedef enum : unsigned char {
typedef enum : uint8_t {
MESH_START, // Prior to start of probe
MESH_FINISH, // Following probe of all points
PROBE_START, // Beginning probe of grid location
Expand Down Expand Up @@ -302,8 +302,8 @@ namespace ExtUI {
FORCE_INLINE uint32_t safe_millis() { return millis(); } // TODO: Implement for AVR
#endif

void delay_us(unsigned long us);
void delay_ms(unsigned long ms);
void delay_us(uint32_t us);
void delay_ms(uint32_t ms);
void yield();

/**
Expand Down
49 changes: 49 additions & 0 deletions Marlin/src/libs/autoreport.h
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
/**
* Marlin 3D Printer Firmware
* Copyright (c) 2020 MarlinFirmware [https://github.com/MarlinFirmware/Marlin]
*
* Based on Sprinter and grbl.
* Copyright (c) 2011 Camiel Gubbels / Erik van der Zalm
*
* This program is free software: you can redistribute it and/or modify
* it under the terms of the GNU General Public License as published by
* the Free Software Foundation, either version 3 of the License, or
* (at your option) any later version.
*
* This program is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
* GNU General Public License for more details.
*
* You should have received a copy of the GNU General Public License
* along with this program. If not, see <https://www.gnu.org/licenses/>.
*
*/
#pragma once

#include "../inc/MarlinConfig.h"

template<serial_index_t AR_PORT_INDEX>
class AutoReporter {
public:
millis_t next_report_ms;
uint8_t report_interval;

// Override this method
inline void auto_report() { }

inline void set_interval(uint8_t seconds, const uint8_t limit=60) {
report_interval = _MIN(seconds, limit);
next_report_ms = millis() + SEC_TO_MS(seconds);
}

inline void tick() {
if (!report_interval) return;
const millis_t ms = millis();
if (ELAPSED(ms, next_report_ms)) {
next_report_ms = ms + SEC_TO_MS(report_interval);
PORT_REDIRECT(AR_PORT_INDEX);
Copy link
Contributor

@X-Ryl669 X-Ryl669 Jan 30, 2021

Choose a reason for hiding this comment

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

Beware, since the serial hooking code, PORT_REDIRECT expect a mask, not an index, you should use PORT_REDIRECT(SERIAL_MASK(AR_PORT_INDEX)) here instead.
There is no need for template parameter here, the mask is a runtime variable, it can be changed anytime!

auto_report();
}
}
};
28 changes: 10 additions & 18 deletions Marlin/src/module/temperature.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1276,7 +1276,7 @@ void Temperature::manage_heater() {
// temperature didn't drop at least MIN_COOLING_SLOPE_DEG_CHAMBER_VENT
if (next_cool_check_ms_2 == 0 || ELAPSED(ms, next_cool_check_ms_2)) {
if (old_temp - temp_chamber.celsius < float(MIN_COOLING_SLOPE_DEG_CHAMBER_VENT)) flag_chamber_excess_heat = true; //the bed is heating the chamber too much
next_cool_check_ms_2 = ms + 1000UL * MIN_COOLING_SLOPE_TIME_CHAMBER_VENT;
next_cool_check_ms_2 = ms + SEC_TO_MS(MIN_COOLING_SLOPE_TIME_CHAMBER_VENT);
old_temp = temp_chamber.celsius;
}
}
Expand Down Expand Up @@ -3123,20 +3123,12 @@ void Temperature::tick() {
}

#if ENABLED(AUTO_REPORT_TEMPERATURES)

uint8_t Temperature::auto_report_temp_interval;
millis_t Temperature::next_temp_report_ms;

void Temperature::auto_report_temperatures() {
if (auto_report_temp_interval && ELAPSED(millis(), next_temp_report_ms)) {
next_temp_report_ms = millis() + 1000UL * auto_report_temp_interval;
PORT_REDIRECT(SERIAL_ALL);
print_heater_states(active_extruder);
SERIAL_EOL();
}
Temperature::AutoReportTemp Temperature::auto_reporter;
void Temperature::AutoReportTemp::auto_report() {
print_heater_states(active_extruder);
SERIAL_EOL();
}

#endif // AUTO_REPORT_TEMPERATURES
#endif

#if HAS_HOTEND && HAS_DISPLAY
void Temperature::set_heating_message(const uint8_t e) {
Expand Down Expand Up @@ -3252,7 +3244,7 @@ void Temperature::tick() {
// if the temperature did not drop at least MIN_COOLING_SLOPE_DEG
if (!next_cool_check_ms || ELAPSED(now, next_cool_check_ms)) {
if (old_temp - temp < float(MIN_COOLING_SLOPE_DEG)) break;
next_cool_check_ms = now + 1000UL * MIN_COOLING_SLOPE_TIME;
next_cool_check_ms = now + SEC_TO_MS(MIN_COOLING_SLOPE_TIME);
old_temp = temp;
}
}
Expand Down Expand Up @@ -3377,7 +3369,7 @@ void Temperature::tick() {
// if the temperature did not drop at least MIN_COOLING_SLOPE_DEG_BED
if (!next_cool_check_ms || ELAPSED(now, next_cool_check_ms)) {
if (old_temp - temp < float(MIN_COOLING_SLOPE_DEG_BED)) break;
next_cool_check_ms = now + 1000UL * MIN_COOLING_SLOPE_TIME_BED;
next_cool_check_ms = now + SEC_TO_MS(MIN_COOLING_SLOPE_TIME_BED);
old_temp = temp;
}
}
Expand Down Expand Up @@ -3461,7 +3453,7 @@ void Temperature::tick() {
SERIAL_ECHOLNPGM("Timed out waiting for probe temperature.");
break;
}
next_delta_check_ms = now + 1000UL * MIN_DELTA_SLOPE_TIME_PROBE;
next_delta_check_ms = now + SEC_TO_MS(MIN_DELTA_SLOPE_TIME_PROBE);
old_temp = temp;
}

Expand Down Expand Up @@ -3566,7 +3558,7 @@ void Temperature::tick() {
// if the temperature did not drop at least MIN_COOLING_SLOPE_DEG_CHAMBER
if (!next_cool_check_ms || ELAPSED(now, next_cool_check_ms)) {
if (old_temp - temp < float(MIN_COOLING_SLOPE_DEG_CHAMBER)) break;
next_cool_check_ms = now + 1000UL * MIN_COOLING_SLOPE_TIME_CHAMBER;
next_cool_check_ms = now + SEC_TO_MS(MIN_COOLING_SLOPE_TIME_CHAMBER);
old_temp = temp;
}
}
Expand Down
14 changes: 6 additions & 8 deletions Marlin/src/module/temperature.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,10 @@
#include "../feature/power.h"
#endif

#if ENABLED(AUTO_REPORT_TEMPERATURES)
#include "../libs/autoreport.h"
#endif

#ifndef SOFT_PWM_SCALE
#define SOFT_PWM_SCALE 0
#endif
Expand Down Expand Up @@ -794,14 +798,8 @@ class Temperature {
#endif
);
#if ENABLED(AUTO_REPORT_TEMPERATURES)
static uint8_t auto_report_temp_interval;
static millis_t next_temp_report_ms;
static void auto_report_temperatures();
static inline void set_auto_report_interval(uint8_t v) {
NOMORE(v, 60);
auto_report_temp_interval = v;
next_temp_report_ms = millis() + 1000UL * v;
}
class AutoReportTemp : public AutoReporter<SERIAL_ALL> { void auto_report(); };
Copy link
Contributor

@X-Ryl669 X-Ryl669 Jan 30, 2021

Choose a reason for hiding this comment

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

auto_report is not virtual so this is never called when manipulating the base class.

Copy link
Contributor

@X-Ryl669 X-Ryl669 Jan 30, 2021

Choose a reason for hiding this comment

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

If you want to avoid the cost of a virtual table (since the instance is known at compile time), you can use CRTP here:

template <class Child>
struct AutoReporter 
{
[...]
inline void auto_report() { static_cast<Child*>(this)->report(); } // Call the child's version. Does not need to be called auto_report
[...]
};

// You need a default implementation that does not report.
struct NoAutoReport : public AutoReporter<NoAutoReport> { void report() {} };

// Then subclass like this:
struct AutoReportTemp : public AutoReporter<AutoReportTemp> { void report() { ... } };

But this means you'll have to typedef the actual class to use since the type will change on each platform:

// In some common header, select the type to use based on compile time information:
#ifdef HAS_MULTI_SERIAL
  typedef AutoReportMulti AutoReport;
#else if ...
  typedef NoAutoReport AutoReport;
[...]
#endif
// Ok, let's declare the reporter instance
extern AutoReport auto_reporter;

static AutoReportTemp auto_reporter;
#endif
#endif

Expand Down
19 changes: 4 additions & 15 deletions Marlin/src/sd/cardreader.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1226,21 +1226,10 @@ void CardReader::fileHasFinished() {
}

#if ENABLED(AUTO_REPORT_SD_STATUS)
uint8_t CardReader::auto_report_sd_interval = 0;
millis_t CardReader::next_sd_report_ms;
#if HAS_MULTI_SERIAL
serial_index_t CardReader::auto_report_port;
#endif

void CardReader::auto_report_sd_status() {
millis_t current_ms = millis();
if (auto_report_sd_interval && ELAPSED(current_ms, next_sd_report_ms)) {
next_sd_report_ms = current_ms + 1000UL * auto_report_sd_interval;
PORT_REDIRECT(auto_report_port);
report_status();
}
}
#endif // AUTO_REPORT_SD_STATUS
TERN_(HAS_MULTI_SERIAL, serial_index_t CardReader::auto_report_port);
CardReader::AutoReportSD CardReader::auto_reporter;
void CardReader::AutoReportSD::auto_report() { report_status(); }
#endif

#if ENABLED(POWER_LOSS_RECOVERY)

Expand Down
32 changes: 14 additions & 18 deletions Marlin/src/sd/cardreader.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,10 @@ typedef struct {
;
} card_flags_t;

#if ENABLED(AUTO_REPORT_SD_STATUS)
#include "../libs/autoreport.h"
#endif

class CardReader {
public:
static card_flags_t flag; // Flags (above)
Expand Down Expand Up @@ -172,13 +176,16 @@ class CardReader {
static Sd2Card& getSd2Card() { return sd2card; }

#if ENABLED(AUTO_REPORT_SD_STATUS)
static void auto_report_sd_status();
static inline void set_auto_report_interval(uint8_t v) {
TERN_(HAS_MULTI_SERIAL, auto_report_port = multiSerial.portMask);
NOMORE(v, 60);
auto_report_sd_interval = v;
next_sd_report_ms = millis() + 1000UL * v;
}
//
// SD Auto Reporting
//
#if HAS_MULTI_SERIAL
static serial_index_t auto_report_port;
Copy link
Contributor

@X-Ryl669 X-Ryl669 Jan 30, 2021

Choose a reason for hiding this comment

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

This is wrong as a static variable is not a constexpr so it can't be used as a template parameter.

#else
static constexpr serial_index_t auto_report_port = 0;
#endif
class AutoReportSD : public AutoReporter<auto_report_port> { void auto_report(); };
static AutoReportSD auto_reporter;
#endif
Copy link
Contributor

@X-Ryl669 X-Ryl669 Jan 30, 2021

Choose a reason for hiding this comment

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

I think, there is no need for a template AutoReporter here, just set the serial_index_t as a member of the class since it's only used for PORT_REDIRECT which is a runtime check, and you can also remove the #if HAS_MULTI_SERIAL condition here so it'll be simpler.


private:
Expand Down Expand Up @@ -260,17 +267,6 @@ class CardReader {
static char proc_filenames[SD_PROCEDURE_DEPTH][MAXPATHNAMELENGTH];
#endif

//
// SD Auto Reporting
//
#if ENABLED(AUTO_REPORT_SD_STATUS)
static uint8_t auto_report_sd_interval;
static millis_t next_sd_report_ms;
#if HAS_MULTI_SERIAL
static serial_index_t auto_report_port;
#endif
#endif

//
// Directory items
//
Expand Down