Code cleanup spi #119
Code cleanup spi #119
Conversation
- Renamed hello_world to starter_example since it clashes with IDF's component name. - Changed NULL to nullptr. - Use operator[] instead of at() since there should be no need to do index-checking. - Use std::for_each instead of indexed loops. - Updated README.md.
| @@ -204,43 +202,45 @@ namespace smooth::application::display | |||
| // stack; we need this memory even when this function is finished because | |||
| // the SPI driver needs access to it even while we're already calculating | |||
| // the next line. | |||
| static std::array<spi_transaction_t, 6> trans; | |||
| static std::array<spi_transaction_t, 6> trans{}; | |||
There was a problem hiding this comment.
@enelson1001 I can't get the comment block below to make sense. trans is not on the stack, so we really shouldn't need to initialize it each time, right?
There was a problem hiding this comment.
Sorry for the delay I am in the USA and this week is Thanksgiving.
To be honest I just copied from esp-idf/examples/spi_master/main/spi_master_example_main.c without digging into the code. I can see that some of the trans can be initialized once but x1, x2, y1,y2, data and pre and post trans states would still need to be set every time.
- Seems better to remove comment. What would you like me to do?
- I updated my fork with git fetch upstream and did a git checkout code-cleanup-SPI. I tested the ili9341 with my M5Stack Demo 1 program and everything thing is still working.
- I have some other questions but will wait until we get your current question resolved.
There was a problem hiding this comment.
Ok, I'll give it some thought. There is no rush, enjoy your stay.
There was a problem hiding this comment.
I ended up simply removing comment.
Use operator[] instead of at().
Use constant.
|
1. If you do another commit - I found that files.cmake has Input.h in the
file twice.
2. I have ordered more hardware and maybe end of the year or in January I
would like to do pull requests for a monochrome display used on M5Stick, a
color display used on M5StickC and for a DHT12 sensor. Would that be OK
for these items to be submitted for a pull-requests?
3. For my education why do you prefer vector/array[item] over vector/
array.at(item). Is there a situation where you would use the.at(item)?
…On Sun, Dec 8, 2019 at 4:57 AM Per Malmberg ***@***.***> wrote:
***@***.**** commented on this pull request.
------------------------------
In lib/smooth/application/display/ILI9341.cpp
<#119 (comment)>:
> @@ -204,43 +202,45 @@ namespace smooth::application::display
// stack; we need this memory even when this function is finished because
// the SPI driver needs access to it even while we're already calculating
// the next line.
- static std::array<spi_transaction_t, 6> trans;
+ static std::array<spi_transaction_t, 6> trans{};
I ended up simply removing comment.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#119?email_source=notifications&email_token=AKLXR724RUBUCAEBLZMDPYTQXTOLZA5CNFSM4JTHCEO2YY3PNVWWK3TUL52HS4DFWFIHK3DMKJSXC5LFON2FEZLWNFSXPKTDN5WW2ZLOORPWSZGOCOLBTRQ#discussion_r355180848>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AKLXR743IGUKCBX42YLGYMTQXTOLZANCNFSM4JTHCEOQ>
.
|
|
If the code is general enough for use outside the M5 stack, then I'll welcome the PR.
|
@enelson1001 I've made some adjustments to the code after merging your PR into master. It's mainly C++-ifying it a bit. Would be great if you can have a look and try it out on your display as I don't have one myself.
Simply checkout the code-cleanup-SPI branch of Smooth.