-
Notifications
You must be signed in to change notification settings - Fork 32
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
Fix : function name for acceleration and deceleration getters #39
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
please fix typo in word "deceleration". Then It should be good to merge
src/ESP_FlexyStepper.cpp
Outdated
return acceleration_InStepsPerSecondPerSecond / stepsPerMillimeter; | ||
} | ||
|
||
float ESP_FlexyStepper::getConfiguredDescelerationInStepsPerSecondPerSecond() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
please fix typo, should be "deceleration"
src/ESP_FlexyStepper.cpp
Outdated
return deceleration_InStepsPerSecondPerSecond; | ||
} | ||
|
||
float ESP_FlexyStepper::getConfiguredDescelerationInRevolutionsPerSecondPerSecond() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
please fix typo, should be "deceleration"
src/ESP_FlexyStepper.cpp
Outdated
return deceleration_InStepsPerSecondPerSecond / stepsPerRevolution; | ||
} | ||
|
||
float ESP_FlexyStepper::getConfiguredDescelerationInMillimetersPerSecondPerSecond() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
please fix typo, should be "deceleration"
src/ESP_FlexyStepper.h
Outdated
float getConfiguredAccelerationInRevolutionsPerSecondPerSecond(); | ||
float getConfiguredAccelerationInMillimetersPerSecondPerSecond(); | ||
|
||
float getConfiguredDescelerationInStepsPerSecondPerSecond(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
please fix typo, should be "deceleration"
Hello, Thanks for the review |
Thanks @ameisso, the typos are fixed, but in commit 8239eb8 you additional changes to the code which are still not fixed according to comment. Also this additional change actually does not match the title of this pull request also. I don't mind so much about the title, but the logic should be changed according to comment to prevent dead code / empty code blocks.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
see comment about your newly added code block for wdt disabling
src/ESP_FlexyStepper.cpp
Outdated
@@ -96,7 +96,9 @@ bool ESP_FlexyStepper::startAsService(int coreNumber) | |||
|
|||
if (coreNumber == 1) | |||
{ | |||
#if ! (CONFIG_IDF_TARGET_ESP32C3 || CONFIG_IDF_TARGET_ESP32S2) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In such a case the whole if statement in the previous line would not make sense, since it would result in an empty block.
Please refactor so that we do not have an empty if block at all. Might need to simply exchange the if / else order.
At the end on such MCUs the function should return false if 1 is given as core number
Hello, changed the block as you asked . |
No description provided.