[rtttl] Various optimizations/clean-up #8923
Conversation
|
Hey there @glmnet, mind taking a look at this pull request as it has been labeled with an integration ( |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## dev #8923 +/- ##
===========================================
- Coverage 53.70% 14.34% -39.37%
===========================================
Files 50 1158 +1108
Lines 9408 48077 +38669
Branches 1654 5774 +4120
===========================================
+ Hits 5053 6898 +1845
- Misses 4056 40741 +36685
- Partials 299 438 +139 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
|
||
| // it's somewhat documented to be up to 10 characters but let's be a bit flexible here | ||
| if (this->position_ == std::string::npos || this->position_ > 15) { | ||
| ESP_LOGE(TAG, "Missing ':' when looking for name."); | ||
| ESP_LOGE(TAG, "Unable to determine name; missing ':'"); |
There was a problem hiding this comment.
IMO ESPHome doesn't really need to have any kind of length restrictions on the name, but that is not for this PR to change.
There was a problem hiding this comment.
I agree on the first part, i will create a PR to resolve this.
Anyway thinking about it is it really an Error for us when there is no name set?
| @@ -387,7 +387,7 @@ static const LogString *state_to_string(State state) { | |||
| void Rtttl::set_state_(State state) { | |||
| State old_state = this->state_; | |||
| this->state_ = state; | |||
| ESP_LOGD(TAG, "State changed from %s to %s", LOG_STR_ARG(state_to_string(old_state)), | |||
| ESP_LOGV(TAG, "State changed from %s to %s", LOG_STR_ARG(state_to_string(old_state)), | |||
There was a problem hiding this comment.
This could also have been set to VV imho. :D
|
Thanks @kbx81 to look into this PR. Did also tried it out? |
What does this implement/fix?
The focus of this PR is some minor code clean-up but more importantly on making log messages shorter and more concise with the goal of making the final binary smaller (more on this to come...).
Types of changes
Related issue or feature (if applicable):
Pull request in esphome-docs with documentation (if applicable):
Test Environment
Example entry for
config.yaml:# Example config.yamlChecklist:
tests/folder).If user exposed functionality or configuration variables are added/changed: