Test of RGBW Branch #660
Test of RGBW Branch #660
Conversation
| { | ||
| _tempEffect = effect; | ||
| } | ||
|
|
There was a problem hiding this comment.
I could swear this functionality existed for the atomic lamp already, but could not find a way to actually set the temp effect, so added this method. Let me know if I missed something that would have obviated the need for it!
There was a problem hiding this comment.
As far as I can remember, the _tempEffect could only be set through the constructor the whole time. But there is a theoretical chance that the possibility did exist a long time ago, and I removed it because the function wasn't used anywhere - I have done some dead code clean-up over time.
| // we'll be showing whole units | ||
| const int precision = size >= threshold ? 2 : 0; | ||
| // If the size is above the threshold, we want a precision of 2 to show more accurate value | ||
| const int precision = size < threshold ? 0 : 2; |
There was a problem hiding this comment.
Check me here, I could be crazy, but I think this was backwards, so flipping it
There was a problem hiding this comment.
I think I'm neutral on the matter.
I am bound to remember this exchange if you change your mind about this in a couple of years, though. ;)
| else | ||
| { | ||
| setPixelOnAllChannels(iStart + i, rgb); | ||
| } | ||
| for (int q = 1; q < everyNth; q++) |
There was a problem hiding this comment.
This seems weird, like every pixel it draws the black ones in between? No idea, but am trying not to change too much here, but this might be a bug worth tracking and/or fixing.
There was a problem hiding this comment.
To the extent I can trace things back (and trust GitHub's blame history), this code was originally submitted by yourself three years ago, probably when this repo was created.
With which I just want to say that you're probably most qualified to have an opinion on this/decide changes are necessary.
There was a problem hiding this comment.
It makes sense now - it fills in the. pixels that the first loop did not!
| _lastPeak1Time[i] = millis(); | ||
| } | ||
| if (_Peaks[i] > _peak2Decay[i]) | ||
| { | ||
| _peak2Decay[i] = _Peaks[i]; | ||
| _peak2Decay[i] = std::min(_Peaks[i], _peak2Decay[i] + maxIncrease); | ||
| } |
There was a problem hiding this comment.
In a later checkin I fix this to use the individual decay rates 1 and 2 rather than basing them both off 1, but it doesn't impact the display.
| { | ||
| _tempEffect = effect; | ||
| } | ||
|
|
There was a problem hiding this comment.
As far as I can remember, the _tempEffect could only be set through the constructor the whole time. But there is a theoretical chance that the possibility did exist a long time ago, and I removed it because the function wasn't used anywhere - I have done some dead code clean-up over time.
| // we'll be showing whole units | ||
| const int precision = size >= threshold ? 2 : 0; | ||
| // If the size is above the threshold, we want a precision of 2 to show more accurate value | ||
| const int precision = size < threshold ? 0 : 2; |
There was a problem hiding this comment.
I think I'm neutral on the matter.
I am bound to remember this exchange if you change your mind about this in a couple of years, though. ;)
| else | ||
| { | ||
| setPixelOnAllChannels(iStart + i, rgb); | ||
| } | ||
| for (int q = 1; q < everyNth; q++) |
There was a problem hiding this comment.
To the extent I can trace things back (and trust GitHub's blame history), this code was originally submitted by yourself three years ago, probably when this repo was created.
With which I just want to say that you're probably most qualified to have an opinion on this/decide changes are necessary.
| return ptrSampleBuffer.get(); | ||
| } | ||
|
|
||
| const size_t GetSampleBufferSize() const |
There was a problem hiding this comment.
Can't this be a constexpr function?
| _lastPeak1Time[i] = millis(); | ||
| } | ||
| if (_Peaks[i] > _peak2Decay[i]) | ||
| { | ||
| _peak2Decay[i] = _Peaks[i]; | ||
| _peak2Decay[i] = std::min(_Peaks[i], _peak2Decay[i] + maxIncrease); | ||
| } |
|
I'm noting for the record that I've re-enabled the CI build for "heltecv3demo". The problem in PlatformIO that caused the project's build to fail was fixed in platform-espressif32 version 6.9.0. |
Description
Adds RGBW support, tweaks SpiralLamp, adds PlateCover
Contributing requirements
mainas the target branch.