refactor: share step targets between remote and group steps

A received step and a group step each worked out where a step takes the
shade, the group copy with its own inline maths. stepTargets() in
ShadeMotion now does it for both: a tilt-only shade steps its tilt, an
integrated tilt turns its slats to the end first and then steps the lift,
and every other shade steps its lift.

Only edge cases change. A group step on a tilt-only shade already at the
end now goes through like a remote step, and a step that can't be sized
(no travel time) leaves the targets alone in both.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
cjkas 2026-10-13 21:17:02 +02:00
parent 5f26340d06
commit c75b715b3b
5 changed files with 160 additions and 134 deletions

View file

@ -36,6 +36,47 @@ bool stepPosition(float pos, int8_t dir, uint32_t travelMs, uint16_t stepSize, u
out = dir > 0 ? std::min(100.0f, pos + step) : std::max(0.0f, pos - step); out = dir > 0 ? std::min(100.0f, pos + step) : std::max(0.0f, pos - step);
return true; return true;
} }
StepTargets stepTargets(tilt_types tilt, int8_t dir, float pos, float tiltPos,
uint32_t upTime, uint32_t downTime, uint32_t tiltTime, uint16_t stepSize, uint8_t frameStepSize) {
StepTargets t;
if(stepSize == 0) return t;
auto atEnd = [dir](float p) { return dir > 0 ? p >= 100.0f : p <= 0.0f; };
const uint32_t liftTime = dir > 0 ? downTime : upTime;
float step;
if(tilt == tilt_types::integrated) {
if(atEnd(tiltPos) && atEnd(pos)) return t;
if(!atEnd(tiltPos)) {
if(!stepPosition(tiltPos, dir, tiltTime, stepSize, frameStepSize, step)) return t;
t.setLift = true;
t.lift = pos;
t.setTilt = true;
t.tilt = step;
}
else {
if(!stepPosition(pos, dir, liftTime, stepSize, frameStepSize, step)) return t;
t.setLift = true;
t.lift = step;
t.setTilt = true;
t.tilt = tiltPos;
}
}
else if(tilt == tilt_types::tiltonly) {
if(!stepPosition(tiltPos, dir, tiltTime, stepSize, frameStepSize, step)) return t;
t.setTilt = true;
t.tilt = step;
}
else {
if(atEnd(pos)) {
t.action = StepAction::none;
return t;
}
if(!stepPosition(pos, dir, liftTime, stepSize, frameStepSize, step)) return t;
t.setLift = true;
t.lift = step;
}
t.action = StepAction::move;
return t;
}
void TxHold::mark(uint32_t ticket, bool moving, uint32_t now) { void TxHold::mark(uint32_t ticket, bool moving, uint32_t now) {
if(ticket == 0 || this->ticket != 0 || moving) return; if(ticket == 0 || this->ticket != 0 || moving) return;
this->ticket = ticket; this->ticket = ticket;

View file

@ -2,6 +2,7 @@
#define SHADE_MOTION_H #define SHADE_MOTION_H
#include <Arduino.h> #include <Arduino.h>
#include "TxBuffer.h" #include "TxBuffer.h"
#include "ShadeTypes.h"
// Shade position maths. Kept free of the shade state, radio and network so it // Shade position maths. Kept free of the shade state, radio and network so it
// can be unit tested on the PC (pio test -e native). // can be unit tested on the PC (pio test -e native).
@ -20,6 +21,25 @@ float limitTarget(int8_t dir);
// ms of the travelMs a full travel takes in that direction. False when // ms of the travelMs a full travel takes in that direction. False when
// travelMs or stepSize is 0, so the step is ignored. // travelMs or stepSize is 0, so the step is ignored.
bool stepPosition(float pos, int8_t dir, uint32_t travelMs, uint16_t stepSize, uint8_t frameStepSize, float &out); bool stepPosition(float pos, int8_t dir, uint32_t travelMs, uint16_t stepSize, uint8_t frameStepSize, float &out);
// What a step command does to a shade.
enum class StepAction : uint8_t {
ignore, // The step can't be sized or the shade is at both ends: drop it.
none, // A lift shade already at the end: nothing moves.
move // Set the targets below.
};
struct StepTargets {
StepAction action = StepAction::ignore;
bool setLift = false;
float lift = 0.0f;
bool setTilt = false;
float tilt = 0.0f;
};
// Where a step in dir (> 0 down, < 0 up) takes a shade at pos and tiltPos. A
// tilt-only shade steps its tilt. An integrated tilt turns its slats to the
// end first, holding the lift, and then steps the lift. Every other shade
// steps its lift.
StepTargets stepTargets(tilt_types tilt, int8_t dir, float pos, float tiltPos,
uint32_t upTime, uint32_t downTime, uint32_t tiltTime, uint16_t stepSize, uint8_t frameStepSize);
// What a held shade does on this pass of checkMovement(). // What a held shade does on this pass of checkMovement().
enum class TxHoldCheck { enum class TxHoldCheck {

View file

@ -136,6 +136,8 @@ class SomfyShade : public SomfyRemote {
void runToLimit(int8_t dir, bool tilt, uint32_t since); void runToLimit(int8_t dir, bool tilt, uint32_t since);
// Sets the targets the sun or wind sends the shade to. // Sets the targets the sun or wind sends the shade to.
void applySensorMove(SensorMove move); void applySensorMove(SensorMove move);
// Sets the targets for a step in dir. False when the step is dropped.
bool applyStep(int8_t dir, uint8_t frameStepSize);
public: public:
uint8_t roomId = 0; uint8_t roomId = 0;
int8_t sortOrder = 0; int8_t sortOrder = 0;

View file

@ -529,7 +529,6 @@ void SomfyShade::processFrame(somfy_frame_t &frame, bool internal) {
const uint64_t curTime = millis(); const uint64_t curTime = millis();
this->lastFrame.copy(frame); this->lastFrame.copy(frame);
int8_t dir = 0; int8_t dir = 0;
float step; // Where a step command takes the shade.
this->moveStart = this->tiltStart = curTime; this->moveStart = this->tiltStart = curTime;
this->startPos = this->currentPos; this->startPos = this->currentPos;
this->startTiltPos = this->currentTiltPos; this->startTiltPos = this->currentTiltPos;
@ -715,40 +714,8 @@ void SomfyShade::processFrame(somfy_frame_t &frame, bool internal) {
this->lastFrame.processed = true; this->lastFrame.processed = true;
if(this->shadeType == shade_types::drycontact || this->shadeType == shade_types::drycontact2) return; if(this->shadeType == shade_types::drycontact || this->shadeType == shade_types::drycontact2) return;
dir = 0; dir = 0;
// With the step commands and integrated shades
// the motor must tilt in the direction first then move
// so we have to calculate the target with this in mind.
if(this->stepSize == 0) return; // Avoid divide by 0.
if(this->lastFrame.stepSize == 0) this->lastFrame.stepSize = 1; if(this->lastFrame.stepSize == 0) this->lastFrame.stepSize = 1;
if(this->tiltType == tilt_types::integrated) { if(!this->applyStep(-1, this->lastFrame.stepSize)) return;
// With integrated tilt this is more involved than ne would think because the step command can be moving not just the tilt
// but the lift. So a determination needs to be made as to whether we are currently moving and it should stop.
// Conditions:
// 1. If both the tilt and lift are at 0% do nothing
// 2. If the tilt position is not currently at the top then shift the tilt.
// 3. If the tilt position is not currently at the top then shift the lift.
if(this->currentTiltPos <= 0.0f && this->currentPos <= 0.0f) return; // Do nothing
else if(this->currentTiltPos > 0.0f) {
// Set the tilt position. This should stop the lift movement.
this->p_target(this->currentPos);
if(!stepPosition(this->currentTiltPos, -1, this->tiltTime, this->stepSize, this->lastFrame.stepSize, step)) return;
this->p_tiltTarget(step);
}
else {
// We only have the lift to move.
if(!stepPosition(this->currentPos, -1, this->upTime, this->stepSize, this->lastFrame.stepSize, step)) return;
this->p_tiltTarget(this->currentTiltPos);
this->p_target(step);
}
}
else if(this->tiltType == tilt_types::tiltonly) {
if(!stepPosition(this->currentTiltPos, -1, this->tiltTime, this->stepSize, this->lastFrame.stepSize, step)) return;
this->p_tiltTarget(step);
}
else if(this->currentPos > 0.0f) {
if(!stepPosition(this->currentPos, -1, this->upTime, this->stepSize, this->lastFrame.stepSize, step)) return;
this->p_target(step);
}
this->emitCommand(cmd, internal ? "internal" : "remote", frame.remoteAddress); this->emitCommand(cmd, internal ? "internal" : "remote", frame.remoteAddress);
break; break;
case somfy_commands::StepDown: case somfy_commands::StepDown:
@ -756,41 +723,8 @@ void SomfyShade::processFrame(somfy_frame_t &frame, bool internal) {
this->lastFrame.processed = true; this->lastFrame.processed = true;
if(this->shadeType == shade_types::drycontact || this->shadeType == shade_types::drycontact2) return; if(this->shadeType == shade_types::drycontact || this->shadeType == shade_types::drycontact2) return;
dir = 1; dir = 1;
// With the step commands and integrated shades
// the motor must tilt in the direction first then move
// so we have to calculate the target with this in mind.
if(this->stepSize == 0) return; // Avoid divide by 0.
if(this->lastFrame.stepSize == 0) this->lastFrame.stepSize = 1; if(this->lastFrame.stepSize == 0) this->lastFrame.stepSize = 1;
if(!this->applyStep(1, this->lastFrame.stepSize)) return;
if(this->tiltType == tilt_types::integrated) {
// With integrated tilt this is more involved than ne would think because the step command can be moving not just the tilt
// but the lift. So a determination needs to be made as to whether we are currently moving and it should stop.
// Conditions:
// 1. If both the tilt and lift are at 100% do nothing
// 2. If the tilt position is not currently at the bottom then shift the tilt.
// 3. If the tilt position is add the bottom then shift the lift.
if(this->currentTiltPos >= 100.0f && this->currentPos >= 100.0f) return; // Do nothing
else if(this->currentTiltPos < 100.0f) {
// Set the tilt position. This should stop the lift movement.
this->p_target(this->currentPos);
if(!stepPosition(this->currentTiltPos, 1, this->tiltTime, this->stepSize, this->lastFrame.stepSize, step)) return;
this->p_tiltTarget(step);
}
else {
// We only have the lift to move.
this->p_tiltTarget(this->currentTiltPos);
if(!stepPosition(this->currentPos, 1, this->downTime, this->stepSize, this->lastFrame.stepSize, step)) return;
this->p_target(step);
}
}
else if(this->tiltType == tilt_types::tiltonly) {
if(!stepPosition(this->currentTiltPos, 1, this->tiltTime, this->stepSize, this->lastFrame.stepSize, step)) return;
this->p_tiltTarget(step);
}
else if(this->currentPos < 100.0f) {
if(!stepPosition(this->currentPos, 1, this->downTime, this->stepSize, this->lastFrame.stepSize, step)) return;
this->p_target(step);
}
this->emitCommand(cmd, internal ? "internal" : "remote", frame.remoteAddress); this->emitCommand(cmd, internal ? "internal" : "remote", frame.remoteAddress);
break; break;
case somfy_commands::Toggle: case somfy_commands::Toggle:
@ -896,75 +830,12 @@ void SomfyShade::processInternalCommand(somfy_commands cmd, uint8_t repeat) {
} }
break; break;
case somfy_commands::StepUp: case somfy_commands::StepUp:
// With the step commands and integrated shades // A group step is always one step.
// the motor must tilt in the direction first then move if(!this->applyStep(-1, 1)) return;
// so we have to calculate the target with this in mind.
if(this->stepSize == 0) return; // Avoid divide by 0.
if(this->tiltType == tilt_types::integrated) {
// With integrated tilt this is more involved than ne would think because the step command can be moving not just the tilt
// but the lift. So a determination needs to be made as to whether we are currently moving and it should stop.
// Conditions:
// 1. If both the tilt and lift are at 0% do nothing
// 2. If the tilt position is not currently at the top then shift the tilt.
// 3. If the tilt position is not currently at the top then shift the lift.
if(this->currentTiltPos <= 0.0f && this->currentPos <= 0.0f) return; // Do nothing
else if(this->currentTiltPos > 0.0f) {
// Set the tilt position. This should stop the lift movement.
this->p_target(this->currentPos);
if(this->tiltTime == 0) return; // Avoid divide by 0.
this->p_tiltTarget(max(0.0f, this->currentTiltPos - (100.0f/(static_cast<float>(this->tiltTime/static_cast<float>(this->stepSize))))));
}
else {
// We only have the lift to move.
if(this->upTime == 0) return; // Avoid divide by 0.
this->p_tiltTarget(this->currentTiltPos);
this->p_target(max(0.0f, this->currentPos - (100.0f/(static_cast<float>(this->upTime/static_cast<float>(this->stepSize))))));
}
}
else if(this->tiltType == tilt_types::tiltonly) {
if(this->tiltTime == 0 || this->currentTiltPos <= 0.0f) return;
this->p_tiltTarget(max(0.0f, this->currentTiltPos - (100.0f/(static_cast<float>(this->tiltTime/static_cast<float>(this->stepSize))))));
}
else if(this->currentPos > 0.0f) {
if(this->upTime == 0) return;
this->p_target(max(0.0f, this->currentPos - (100.0f/(static_cast<float>(this->upTime/static_cast<float>(this->stepSize))))));
}
break; break;
case somfy_commands::StepDown: case somfy_commands::StepDown:
dir = 1; dir = 1;
// With the step commands and integrated shades if(!this->applyStep(1, 1)) return;
// the motor must tilt in the direction first then move
// so we have to calculate the target with this in mind.
if(this->stepSize == 0) return; // Avoid divide by 0.
if(this->tiltType == tilt_types::integrated) {
// With integrated tilt this is more involved than ne would think because the step command can be moving not just the tilt
// but the lift. So a determination needs to be made as to whether we are currently moving and it should stop.
// Conditions:
// 1. If both the tilt and lift are at 100% do nothing
// 2. If the tilt position is not currently at the bottom then shift the tilt.
// 3. If the tilt position is add the bottom then shift the lift.
if(this->currentTiltPos >= 100.0f && this->currentPos >= 100.0f) return; // Do nothing
else if(this->currentTiltPos < 100.0f) {
// Set the tilt position. This should stop the lift movement.
this->p_target(this->currentPos);
if(this->tiltTime == 0) return; // Avoid divide by 0.
this->p_tiltTarget(min(100.0f, this->currentTiltPos + (100.0f/(static_cast<float>(this->tiltTime/static_cast<float>(this->stepSize))))));
}
else {
// We only have the lift to move.
if(this->downTime == 0) return; // Avoid divide by 0.
this->p_tiltTarget(this->currentTiltPos);
this->p_target(min(100.0f, this->currentPos + (100.0f/(static_cast<float>(this->downTime/static_cast<float>(this->stepSize))))));
}
}
else if(this->tiltType == tilt_types::tiltonly) {
if(this->tiltTime == 0 || this->stepSize == 0 || this->currentTiltPos >= 100.0f) return;
this->p_tiltTarget(min(100.0f, this->currentTiltPos + (100.0f/(static_cast<float>(this->tiltTime/static_cast<float>(this->stepSize))))));
}
else if(this->currentPos < 100.0f) {
if(this->downTime == 0 || this->stepSize == 0) return;
this->p_target(min(100.0f, this->currentPos + (100.0f/(static_cast<float>(this->downTime/static_cast<float>(this->stepSize))))));
}
break; break;
case somfy_commands::Flag: case somfy_commands::Flag:
this->p_sunFlag(false); this->p_sunFlag(false);
@ -1097,6 +968,14 @@ void SomfyShade::applySensorMove(SensorMove move) {
if(t.setLift) this->p_target(t.lift); if(t.setLift) this->p_target(t.lift);
if(t.setTilt) this->p_tiltTarget(t.tilt); if(t.setTilt) this->p_tiltTarget(t.tilt);
} }
bool SomfyShade::applyStep(int8_t dir, uint8_t frameStepSize) {
const StepTargets t = stepTargets(this->tiltType, dir, this->currentPos, this->currentTiltPos,
this->upTime, this->downTime, this->tiltTime, this->stepSize, frameStepSize);
if(t.action == StepAction::ignore) return false;
if(t.setLift) this->p_target(t.lift);
if(t.setTilt) this->p_tiltTarget(t.tilt);
return true;
}
void SomfyShade::setMovement(int8_t dir) { void SomfyShade::setMovement(int8_t dir) {
int8_t currDir = this->direction; int8_t currDir = this->direction;
int8_t currTiltDir = this->tiltDirection; int8_t currTiltDir = this->tiltDirection;

View file

@ -210,6 +210,81 @@ static void test_txhold_times_out_across_millis_wrap() {
TEST_ASSERT_TRUE(hold.timedOut(0x00007000u, 30000)); // 32768ms later TEST_ASSERT_TRUE(hold.timedOut(0x00007000u, 30000)); // 32768ms later
} }
/*********************************************************************
* stepTargets
********************************************************************/
// One step of 100ms: 0.5% of the 20s up travel, 1% of the 10s down travel and
// 2% of the 5s tilt travel.
static StepTargets stepOf(tilt_types tilt, int8_t dir, float pos, float tiltPos, uint8_t frameSteps = 1,
uint32_t upTime = 20000, uint32_t downTime = 10000, uint32_t tiltTime = 5000, uint16_t stepSize = 100) {
return stepTargets(tilt, dir, pos, tiltPos, upTime, downTime, tiltTime, stepSize, frameSteps);
}
#define ASSERT_ACTION(expected, actual) TEST_ASSERT_EQUAL_UINT8(static_cast<uint8_t>(expected), static_cast<uint8_t>(actual))
static void test_stepTargets_lift_shade_steps_its_lift() {
StepTargets t = stepOf(tilt_types::none, DOWN, 40, 0);
ASSERT_ACTION(StepAction::move, t.action);
TEST_ASSERT_TRUE(t.setLift);
ASSERT_POS(41, t.lift);
TEST_ASSERT_FALSE(t.setTilt);
// Up uses the up travel time.
t = stepOf(tilt_types::none, UP, 40, 0);
ASSERT_POS(39.5f, t.lift);
}
static void test_stepTargets_frame_steps_multiply_the_step() {
ASSERT_POS(43, stepOf(tilt_types::none, DOWN, 40, 0, 3).lift);
}
static void test_stepTargets_tilt_motors_step_the_lift() {
// A separate tilt motor or euromode shade tilts with a long press, not a step.
ASSERT_POS(41, stepOf(tilt_types::tiltmotor, DOWN, 40, 50).lift);
TEST_ASSERT_FALSE(stepOf(tilt_types::euromode, DOWN, 40, 50).setTilt);
}
static void test_stepTargets_lift_shade_at_the_end_has_nothing_to_do() {
ASSERT_ACTION(StepAction::none, stepOf(tilt_types::none, DOWN, 100, 0).action);
ASSERT_ACTION(StepAction::none, stepOf(tilt_types::none, UP, 0, 0).action);
}
static void test_stepTargets_tilt_only_shade_steps_its_tilt() {
StepTargets t = stepOf(tilt_types::tiltonly, DOWN, 100, 40);
ASSERT_ACTION(StepAction::move, t.action);
TEST_ASSERT_FALSE(t.setLift);
TEST_ASSERT_TRUE(t.setTilt);
ASSERT_POS(42, t.tilt);
// At the end the tilt stays there.
t = stepOf(tilt_types::tiltonly, UP, 100, 0);
ASSERT_ACTION(StepAction::move, t.action);
ASSERT_POS(0, t.tilt);
}
static void test_stepTargets_integrated_tilts_first() {
StepTargets t = stepOf(tilt_types::integrated, DOWN, 20, 40);
ASSERT_ACTION(StepAction::move, t.action);
// The lift holds where it is while the slats turn.
TEST_ASSERT_TRUE(t.setLift);
ASSERT_POS(20, t.lift);
TEST_ASSERT_TRUE(t.setTilt);
ASSERT_POS(42, t.tilt);
}
static void test_stepTargets_integrated_lifts_once_tilted() {
StepTargets t = stepOf(tilt_types::integrated, DOWN, 20, 100);
ASSERT_ACTION(StepAction::move, t.action);
ASSERT_POS(21, t.lift);
ASSERT_POS(100, t.tilt);
t = stepOf(tilt_types::integrated, UP, 50, 0);
ASSERT_POS(49.5f, t.lift);
ASSERT_POS(0, t.tilt);
}
static void test_stepTargets_integrated_at_both_ends_is_ignored() {
ASSERT_ACTION(StepAction::ignore, stepOf(tilt_types::integrated, DOWN, 100, 100).action);
ASSERT_ACTION(StepAction::ignore, stepOf(tilt_types::integrated, UP, 0, 0).action);
}
static void test_stepTargets_without_a_travel_time_or_step_size_is_ignored() {
ASSERT_ACTION(StepAction::ignore, stepOf(tilt_types::none, DOWN, 40, 0, 1, 20000, 0).action);
ASSERT_ACTION(StepAction::ignore, stepOf(tilt_types::none, UP, 40, 0, 1, 0).action);
ASSERT_ACTION(StepAction::ignore, stepOf(tilt_types::tiltonly, DOWN, 100, 40, 1, 20000, 10000, 0).action);
ASSERT_ACTION(StepAction::ignore, stepOf(tilt_types::integrated, DOWN, 20, 40, 1, 20000, 10000, 0).action);
// Even a shade with nothing to do ignores a step it can't size.
ASSERT_ACTION(StepAction::ignore, stepOf(tilt_types::none, DOWN, 100, 0, 1, 20000, 10000, 5000, 0).action);
}
int main(int argc, char **argv) { int main(int argc, char **argv) {
(void)argc; (void)argc;
(void)argv; (void)argv;
@ -227,6 +302,15 @@ int main(int argc, char **argv) {
RUN_TEST(test_step_stops_at_the_ends); RUN_TEST(test_step_stops_at_the_ends);
RUN_TEST(test_step_is_ignored_without_a_travel_time_or_step_size); RUN_TEST(test_step_is_ignored_without_a_travel_time_or_step_size);
RUN_TEST(test_step_of_no_frame_steps_stays_put); RUN_TEST(test_step_of_no_frame_steps_stays_put);
RUN_TEST(test_stepTargets_lift_shade_steps_its_lift);
RUN_TEST(test_stepTargets_frame_steps_multiply_the_step);
RUN_TEST(test_stepTargets_tilt_motors_step_the_lift);
RUN_TEST(test_stepTargets_lift_shade_at_the_end_has_nothing_to_do);
RUN_TEST(test_stepTargets_tilt_only_shade_steps_its_tilt);
RUN_TEST(test_stepTargets_integrated_tilts_first);
RUN_TEST(test_stepTargets_integrated_lifts_once_tilted);
RUN_TEST(test_stepTargets_integrated_at_both_ends_is_ignored);
RUN_TEST(test_stepTargets_without_a_travel_time_or_step_size_is_ignored);
RUN_TEST(test_displayPosition); RUN_TEST(test_displayPosition);
RUN_TEST(test_flipCommand_swaps_directions); RUN_TEST(test_flipCommand_swaps_directions);
RUN_TEST(test_flipCommand_keeps_the_rest); RUN_TEST(test_flipCommand_keeps_the_rest);