diff --git a/docs/KNOWN_ISSUES.md b/docs/KNOWN_ISSUES.md index ae40e6d..9c8bc86 100644 --- a/docs/KNOWN_ISSUES.md +++ b/docs/KNOWN_ISSUES.md @@ -115,16 +115,3 @@ Sending a command reads the next rolling code, queues the frame, then commits th - Same root cause as [Concurrent mutation during chunked `/controller` response](#concurrent-mutation-during-chunked-controller-response): the web server task changes the shade model that `loop()` owns. - `/linkRemote` sets a linked remote's rolling code on the web server task, while `loop()` updates the same code when it receives a frame from that remote. The last write wins, so a received frame can undo the code just set. - -## `/setMyPosition` without a tilt uses the lift favorite - -**Status:** open -**Location:** the `/setMyPosition` handlers in [src/Web.cpp](../src/Web.cpp). This was already in the code before the radio task work. - -### Summary - -When a request for a shade with tilt leaves out `tilt`, the handler fills it in with `shade->myPos`, the lift favorite, where `shade->myTiltPos` looks intended. The favorite tilt is then set to the favorite lift position. The web UI always sends `tilt`, so only API callers that leave it out see this. - -### Fix - -Use `shade->myTiltPos` in both handlers, after checking that no API client depends on the current behaviour. diff --git a/src/ShadeMotion.cpp b/src/ShadeMotion.cpp index 54a8f75..6fd006b 100644 --- a/src/ShadeMotion.cpp +++ b/src/ShadeMotion.cpp @@ -29,6 +29,11 @@ int8_t displayPosition(float pos, bool flip) { if(pos < 0) return -1; return static_cast(flip && pos >= 0.00f ? floor(100.0f - pos) : floor(pos)); } +int8_t requestedFavoriteTilt(int8_t requestTilt, tilt_types tilt, float myTiltPos, bool flip) { + if(tilt == tilt_types::none) return -1; + if(requestTilt >= 0) return requestTilt; + return displayPosition(myTiltPos, flip); +} float limitTarget(int8_t dir) { return dir > 0 ? 100.0f : 0.0f; } bool stepPosition(float pos, int8_t dir, uint32_t travelMs, uint16_t stepSize, uint8_t frameStepSize, float &out) { if(travelMs == 0 || stepSize == 0) return false; diff --git a/src/ShadeMotion.h b/src/ShadeMotion.h index 1fa10bd..0c801ab 100644 --- a/src/ShadeMotion.h +++ b/src/ShadeMotion.h @@ -14,6 +14,10 @@ float travelPosition(float startPos, int8_t dir, uint32_t elapsedMs, int32_t tra // A position as reported to clients: -1 when unset, otherwise the whole // percentage, counted from the other end when flipped. int8_t displayPosition(float pos, bool flip); +// The tilt a request to set the favorite asks for, as a display position: the +// requested tilt, or the favorite tilt when the request leaves it out. -1 for +// a shade without tilt or with no favorite tilt. +int8_t requestedFavoriteTilt(int8_t requestTilt, tilt_types tilt, float myTiltPos, bool flip); // Where a shade (or its tilt) moving in dir ends up when nothing stops it. float limitTarget(int8_t dir); // Where a step command moves a shade (or its tilt) from pos, in out. dir > 0 diff --git a/src/Web.cpp b/src/Web.cpp index 72a4052..d122592 100644 --- a/src/Web.cpp +++ b/src/Web.cpp @@ -1733,8 +1733,7 @@ void Web::begin() { } SomfyShade* shade = somfy.getShadeById(shadeId); if(shade) { - if(tilt < 0) tilt = shade->myPos; - if(shade->tiltType == tilt_types::none) tilt = -1; + tilt = requestedFavoriteTilt(tilt, shade->tiltType, shade->myTiltPos, shade->flipPosition); if(pos >= 0 && pos <= 100) if(!queueSetMyPosition(request, shade, shade->transformPosition(pos), shade->transformPosition(tilt))) return; AsyncJsonResp resp; @@ -1760,8 +1759,7 @@ void Web::begin() { else { request->send(500, _encoding_json, F("{\"status\":\"ERROR\",\"desc\":\"No shade object supplied.\"}")); return; } SomfyShade* shade = somfy.getShadeById(shadeId); if(shade) { - if(tilt < 0) tilt = shade->myPos; - if(shade->tiltType == tilt_types::none) tilt = -1; + tilt = requestedFavoriteTilt(tilt, shade->tiltType, shade->myTiltPos, shade->flipPosition); if(pos >= 0 && pos <= 100) if(!queueSetMyPosition(request, shade, shade->transformPosition(pos), shade->transformPosition(tilt))) return; AsyncJsonResp resp; diff --git a/test/test_motion/test_motion.cpp b/test/test_motion/test_motion.cpp index 6996a42..744147b 100644 --- a/test/test_motion/test_motion.cpp +++ b/test/test_motion/test_motion.cpp @@ -285,6 +285,25 @@ static void test_stepTargets_without_a_travel_time_or_step_size_is_ignored() { ASSERT_ACTION(StepAction::ignore, stepOf(tilt_types::none, DOWN, 100, 0, 1, 20000, 10000, 5000, 0).action); } +/********************************************************************* + * requestedFavoriteTilt + ********************************************************************/ +static void test_favorite_tilt_keeps_the_requested_tilt() { + TEST_ASSERT_EQUAL_INT8(25, requestedFavoriteTilt(25, tilt_types::integrated, 70.0f, false)); +} +static void test_favorite_tilt_defaults_to_the_favorite_tilt() { + // Not the lift favorite: that was the old default. + TEST_ASSERT_EQUAL_INT8(70, requestedFavoriteTilt(-1, tilt_types::tiltmotor, 70.0f, false)); + // The request is in display positions, so a flipped shade gets its favorite flipped. + TEST_ASSERT_EQUAL_INT8(30, requestedFavoriteTilt(-1, tilt_types::tiltmotor, 70.0f, true)); +} +static void test_favorite_tilt_without_a_favorite_is_unset() { + TEST_ASSERT_EQUAL_INT8(-1, requestedFavoriteTilt(-1, tilt_types::integrated, -1.0f, false)); +} +static void test_favorite_tilt_of_a_shade_without_tilt_is_unset() { + TEST_ASSERT_EQUAL_INT8(-1, requestedFavoriteTilt(25, tilt_types::none, 70.0f, false)); +} + int main(int argc, char **argv) { (void)argc; (void)argv; @@ -312,6 +331,10 @@ int main(int argc, char **argv) { 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_favorite_tilt_keeps_the_requested_tilt); + RUN_TEST(test_favorite_tilt_defaults_to_the_favorite_tilt); + RUN_TEST(test_favorite_tilt_without_a_favorite_is_unset); + RUN_TEST(test_favorite_tilt_of_a_shade_without_tilt_is_unset); RUN_TEST(test_flipCommand_swaps_directions); RUN_TEST(test_flipCommand_keeps_the_rest); RUN_TEST(test_txhold_idle_shade_is_held);