Skip to content

Commit

Permalink
♻️ Planner flags refactor
Browse files Browse the repository at this point in the history
  • Loading branch information
thinkyhead committed Jun 28, 2022
1 parent 884f7c7 commit 307dfb1
Show file tree
Hide file tree
Showing 7 changed files with 153 additions and 142 deletions.
2 changes: 1 addition & 1 deletion Marlin/src/gcode/motion/G2_G3.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -315,7 +315,7 @@ void plan_arc(
// Compute exact location by applying transformation matrix from initial radius vector(=-offset).
// To reduce stuttering, the sin and cos could be computed at different times.
// For now, compute both at the same time.
const float cos_Ti = cos(i * theta_per_segment), sin_Ti = sin(i * theta_per_segment);
const float Ti = i * theta_per_segment, cos_Ti = cos(Ti), sin_Ti = sin(Ti);
rvec.a = -offset[0] * cos_Ti + offset[1] * sin_Ti;
rvec.b = -offset[0] * sin_Ti - offset[1] * cos_Ti;
}
Expand Down
2 changes: 1 addition & 1 deletion Marlin/src/gcode/motion/G6.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ void GcodeSuite::G6() {
// No speed is set, can't schedule the move
if (!planner.last_page_step_rate) return;

const page_idx_t page_idx = (page_idx_t) parser.value_ulong();
const page_idx_t page_idx = (page_idx_t)parser.value_ulong();

uint16_t num_steps = DirectStepping::Config::TOTAL_STEPS;
if (parser.seen('S')) num_steps = parser.value_ushort();
Expand Down
4 changes: 2 additions & 2 deletions Marlin/src/gcode/temp/M106_M107.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ void GcodeSuite::M106() {
// Set speed, with constraint
thermalManager.set_fan_speed(pfan, speed);

TERN_(LASER_SYNCHRONOUS_M106_M107, planner.buffer_sync_block(BLOCK_FLAG_SYNC_FANS));
TERN_(LASER_SYNCHRONOUS_M106_M107, planner.buffer_sync_block(BLOCK_BIT_SYNC_FANS));

if (TERN0(DUAL_X_CARRIAGE, idex_is_duplicating())) // pfan == 0 when duplicating
thermalManager.set_fan_speed(1 - pfan, speed);
Expand All @@ -111,7 +111,7 @@ void GcodeSuite::M107() {
if (TERN0(DUAL_X_CARRIAGE, idex_is_duplicating())) // pfan == 0 when duplicating
thermalManager.set_fan_speed(1 - pfan, 0);

TERN_(LASER_SYNCHRONOUS_M106_M107, planner.buffer_sync_block(BLOCK_FLAG_SYNC_FANS));
TERN_(LASER_SYNCHRONOUS_M106_M107, planner.buffer_sync_block(BLOCK_BIT_SYNC_FANS));
}

#endif // HAS_FAN
155 changes: 74 additions & 81 deletions Marlin/src/module/planner.cpp

Large diffs are not rendered by default.

108 changes: 65 additions & 43 deletions Marlin/src/module/planner.h
Original file line number Diff line number Diff line change
Expand Up @@ -70,9 +70,6 @@

#if ENABLED(DIRECT_STEPPING)
#include "../feature/direct_stepping.h"
#define IS_PAGE(B) TEST(B->flag, BLOCK_BIT_IS_PAGE)
#else
#define IS_PAGE(B) false
#endif

#if ENABLED(EXTERNAL_CLOSED_LOOP_CONTROLLER)
Expand All @@ -92,7 +89,34 @@
#define HAS_DIST_MM_ARG 1
#endif

enum BlockFlagBit : char {
#if ENABLED(LASER_POWER_INLINE)

typedef struct {
bool isPlanned:1;
bool isEnabled:1;
bool dir:1;
bool Reserved:6;
} power_status_t;

typedef struct {
power_status_t status; // See planner settings for meaning
uint8_t power; // Ditto; When in trapezoid mode this is nominal power
#if ENABLED(LASER_POWER_INLINE_TRAPEZOID)
uint8_t power_entry; // Entry power for the laser
#if DISABLED(LASER_POWER_INLINE_TRAPEZOID_CONT)
uint8_t power_exit; // Exit power for the laser
uint32_t entry_per, // Steps per power increment (to avoid floats in stepper calcs)
exit_per; // Steps per power decrement
#endif
#endif
} block_laser_t;

#endif

/**
* Planner block flags as boolean bit fields
*/
enum BlockFlagBit {
// Recalculate trapezoids on entry junction. For optimization.
BLOCK_BIT_RECALCULATE,

Expand All @@ -109,7 +133,7 @@ enum BlockFlagBit : char {

// Direct stepping page
#if ENABLED(DIRECT_STEPPING)
, BLOCK_BIT_IS_PAGE
, BLOCK_BIT_PAGE
#endif

// Sync the fan speeds from the block
Expand All @@ -118,57 +142,55 @@ enum BlockFlagBit : char {
#endif
};

enum BlockFlag : char {
BLOCK_FLAG_RECALCULATE = _BV(BLOCK_BIT_RECALCULATE)
, BLOCK_FLAG_NOMINAL_LENGTH = _BV(BLOCK_BIT_NOMINAL_LENGTH)
, BLOCK_FLAG_CONTINUED = _BV(BLOCK_BIT_CONTINUED)
, BLOCK_FLAG_SYNC_POSITION = _BV(BLOCK_BIT_SYNC_POSITION)
#if ENABLED(DIRECT_STEPPING)
, BLOCK_FLAG_IS_PAGE = _BV(BLOCK_BIT_IS_PAGE)
#endif
#if ENABLED(LASER_SYNCHRONOUS_M106_M107)
, BLOCK_FLAG_SYNC_FANS = _BV(BLOCK_BIT_SYNC_FANS)
#endif
};
/**
* Planner block flags as boolean bit fields
*/
typedef struct {
union {
uint8_t bits;

#define BLOCK_MASK_SYNC ( BLOCK_FLAG_SYNC_POSITION | TERN0(LASER_SYNCHRONOUS_M106_M107, BLOCK_FLAG_SYNC_FANS) )
struct {
bool recalculate:1;

#if ENABLED(LASER_POWER_INLINE)
bool nominal_length:1;

typedef struct {
bool isPlanned:1;
bool isEnabled:1;
bool dir:1;
bool Reserved:6;
} power_status_t;
bool continued:1;

typedef struct {
power_status_t status; // See planner settings for meaning
uint8_t power; // Ditto; When in trapezoid mode this is nominal power
#if ENABLED(LASER_POWER_INLINE_TRAPEZOID)
uint8_t power_entry; // Entry power for the laser
#if DISABLED(LASER_POWER_INLINE_TRAPEZOID_CONT)
uint8_t power_exit; // Exit power for the laser
uint32_t entry_per, // Steps per power increment (to avoid floats in stepper calcs)
exit_per; // Steps per power decrement
bool sync_position:1;

#if ENABLED(DIRECT_STEPPING)
bool page:1;
#endif
#endif
} block_laser_t;

#endif
#if ENABLED(LASER_SYNCHRONOUS_M106_M107)
bool sync_fans:1;
#endif
};
};

void clear() volatile { bits = 0; }
void apply(const uint8_t f) volatile { bits |= f; }
void apply(const BlockFlagBit b) volatile { SBI(bits, b); }
void reset(const BlockFlagBit b) volatile { bits = _BV(b); }
void set_nominal(const bool n) volatile { recalculate = true; if (n) nominal_length = true; }

} block_flags_t;

/**
* struct block_t
*
* A single entry in the planner buffer.
* Tracks linear movement over multiple axes.
* A single entry in the planner buffer, used to set up and
* track a coordinated linear motion for one or more axes.
*
* The "nominal" values are as-specified by G-code, and
* may never actually be reached due to acceleration limits.
*/
typedef struct block_t {

volatile uint8_t flag; // Block flags (See BlockFlag enum above) - Modified by ISR and main thread!
volatile block_flags_t flag; // Block flags

volatile bool is_fan_sync() { return TERN0(LASER_SYNCHRONOUS_M106_M107, flag.sync_fans); }
volatile bool is_sync() { return flag.sync_position || is_fan_sync(); }
volatile bool is_page() { return TERN0(DIRECT_STEPPING, flag.page); }
volatile bool is_move() { return !(is_sync() || is_page()); }

// Fields used by the motion planner to manage acceleration
float nominal_speed_sqr, // The nominal speed for this block in (mm/sec)^2
Expand Down Expand Up @@ -759,7 +781,7 @@ class Planner {
* case of LASER_SYNCHRONOUS_M106_M107 the fan pwm
*/
static void buffer_sync_block(
TERN_(LASER_SYNCHRONOUS_M106_M107, uint8_t sync_flag=BLOCK_FLAG_SYNC_POSITION)
TERN_(LASER_SYNCHRONOUS_M106_M107, const BlockFlagBit flag=BLOCK_BIT_SYNC_POSITION)
);

#if IS_KINEMATIC
Expand Down
21 changes: 9 additions & 12 deletions Marlin/src/module/stepper.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1699,7 +1699,7 @@ void Stepper::pulse_phase_isr() {
}while(0)

// Direct Stepping page?
const bool is_page = IS_PAGE(current_block);
const bool is_page = current_block->is_page();

#if ENABLED(DIRECT_STEPPING)
// Direct stepping is currently not ready for HAS_I_AXIS
Expand Down Expand Up @@ -1977,7 +1977,7 @@ uint32_t Stepper::block_phase_isr() {
count_position[_AXIS(AXIS)] += page_step_state.bd[_AXIS(AXIS)] * count_direction[_AXIS(AXIS)];
#endif

if (IS_PAGE(current_block)) {
if (current_block->is_page()) {
PAGE_SEGMENT_UPDATE_POS(X);
PAGE_SEGMENT_UPDATE_POS(Y);
PAGE_SEGMENT_UPDATE_POS(Z);
Expand Down Expand Up @@ -2167,16 +2167,13 @@ uint32_t Stepper::block_phase_isr() {
if ((current_block = planner.get_current_block())) {

// Sync block? Sync the stepper counts or fan speeds and return
while (current_block->flag & BLOCK_MASK_SYNC) {
while (current_block->is_sync()) {

#if ENABLED(LASER_SYNCHRONOUS_M106_M107)
const bool is_sync_fans = TEST(current_block->flag, BLOCK_BIT_SYNC_FANS);
if (is_sync_fans) planner.sync_fan_speeds(current_block->fan_speed);
#else
constexpr bool is_sync_fans = false;
#endif

if (!is_sync_fans) _set_position(current_block->position);
if (current_block->is_fan_sync()) {
TERN_(LASER_SYNCHRONOUS_M106_M107, planner.sync_fan_speeds(current_block->fan_speed));
}
else
_set_position(current_block->position);

discard_current_block();

Expand All @@ -2196,7 +2193,7 @@ uint32_t Stepper::block_phase_isr() {
#endif

#if ENABLED(DIRECT_STEPPING)
if (IS_PAGE(current_block)) {
if (current_block->is_page()) {
page_step_state.segment_steps = 0;
page_step_state.segment_idx = 0;
page_step_state.page = page_manager.get_page(current_block->page_idx);
Expand Down
3 changes: 1 addition & 2 deletions Marlin/src/module/stepper.h
Original file line number Diff line number Diff line change
Expand Up @@ -524,8 +524,7 @@ class Stepper {
// Discard current block and free any resources
FORCE_INLINE static void discard_current_block() {
#if ENABLED(DIRECT_STEPPING)
if (IS_PAGE(current_block))
page_manager.free_page(current_block->page_idx);
if (current_block->is_page()) page_manager.free_page(current_block->page_idx);
#endif
current_block = nullptr;
axis_did_move = 0;
Expand Down

1 comment on commit 307dfb1

@fnsign
Copy link

@fnsign fnsign commented on 307dfb1 Jun 29, 2022

Choose a reason for hiding this comment

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

I get following compiler warning on the bugfix-2.1.x branch:
Marlin/src/module/planner.cpp: In static member function 'static void Planner::buffer_sync_block()': Marlin/src/module/planner.cpp:2899:35: warning: 'void* memset(void*, int, size_t)' clearing an object of type 'block_t' {aka 'struct block_t'} with no trivial copy-assignment; use value-initialization instead [-Wclass-memaccess] 2899 | memset(block, 0, sizeof(block_t)); | ^ In file included from Marlin/src/module/planner.cpp:65: Marlin/src/module/planner.h:186:16: note: 'block_t' {aka 'struct block_t'} declared here 186 | typedef struct block_t { | ^~~~~~~

Printer: Ender 3 Pro with SKR E3 mini v1.2

Please sign in to comment.