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

_currentLevel == _targetLevel not handled properly #30

Open
alexelite opened this issue Jan 19, 2021 · 0 comments
Open

_currentLevel == _targetLevel not handled properly #30

alexelite opened this issue Jan 19, 2021 · 0 comments

Comments

@alexelite
Copy link

alexelite commented Jan 19, 2021

Firstly, thank you @marvinroger for this library.

I did some testing for my custom usage and found 2 situations when state machine wrongly changes from idle to targeting.

Initial condition: All testing is done using the latest github version and with the default example, using Arduino IDE, and an Arduino UNO type board.
The only changes I made was to add 'u' after first number for upCourseTime and downCourseTime definition and setLevel(30) changed to setLevel(0) in setup.

const unsigned long upCourseTime = 30u * 1000;
const unsigned long downCourseTime = 45u * 1000;

I found 2 bad behaviors as folows
Bad behavior 1:
During a reset, if setLevel(0) is called, when reset finishes, state machine switches from RESETTING to IDLE to TARGETING and then into a RESETTING loop because _currentLevel goes outside normal 0-100 range (to 255).
Calling for any other level fixes the loop.
Evaluating _targetLevel == _currentLevel and asigning LEVEL_NONE to _targetLevel if true, keeps the state machine in IDLE.

adding this line in loop function:
if (_targetLevel == _currentLevel) _targetLevel = LEVEL_NONE;
before:
if (_state == STATE_IDLE && _targetLevel == LEVEL_NONE) return *this; // nothing to do
solves the problem

Bad behavior 2:
If during an 100% end run calibration, setLevel(100) is called, after calibration finishes, the state machine goes into TARGETING and goes to 0% and then returns to 100%.
In setLevel function, calibration state is not evaluated and _targetLevel is changed from LEVEL_NONE to 100. State machine switches from IDLE to TARGETING because of this. At this point direction is set to UP and machine moves to 0, IDLES, but still having a target set, starts moving down to 100.
Calling a setLevel during this move fixes the behavior.
Same line as before fixes the problem, but can also be fixed before by changing:

in setLevel function this:
if (_state == STATE_IDLE && level == _currentLevel) return *this;
to:
if (_state == STATE_IDLE || _state == STATE_CALIBRATING) && level == _currentLevel) return *this;

I currently use only the first change, and both problems are gone.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant