I'm so overwhelmed. =/
-
I'm headdesking this code I'm trying to port. It's just a godawful mess. I'm barely treading water, even as I understand about 70% of it more or less, it's really difficult to modify. https://github.com/KurtE/ILI9341\_t3n specifically: https://github.com/KurtE/ILI9341\_t3n/blob/master/src/ILI9341\_t3n.cpp Kurt's a nice guy. I've corresponded with him, not about this code directly, but about my attempt at making similar code that works with htcw_gfx, my graphics library for IoT devices. That said, I hate the way he coded this. I feel like design was either an afterthought, or he just added to it over and over without ever bothering to step back and refactor. I've done that too, so it's not really a judgment of his coding in general, but it *is* difficult to deal with in this case. Now I need to though, because for starters his device reads are not const (if you know C++ then you should understand the ramifications of that) and they aren't easily made const because of the way it's implemented, so if I want to read pixels off the display device I can't do that, because htcw_gfx expects readable things to be able to const. I can fix that by moving all the driver communication code to a static template class whose arguments include the DC and CS pins, so I have statics per SPI device, but it means open heart surgery on Kurt's code, and I'm kind of overwhelmed trying to piece this all together. This is already my second attempt. :sigh: I love building things, but sometimes the act itself stifles the creative part. Like, the inspiration for the creation vs the act of creation itself - sometimes they are at odds. Hebrew has words for each part of that creative process. I wish English did.
To err is human. Fortune favors the monsters.
-
I'm headdesking this code I'm trying to port. It's just a godawful mess. I'm barely treading water, even as I understand about 70% of it more or less, it's really difficult to modify. https://github.com/KurtE/ILI9341\_t3n specifically: https://github.com/KurtE/ILI9341\_t3n/blob/master/src/ILI9341\_t3n.cpp Kurt's a nice guy. I've corresponded with him, not about this code directly, but about my attempt at making similar code that works with htcw_gfx, my graphics library for IoT devices. That said, I hate the way he coded this. I feel like design was either an afterthought, or he just added to it over and over without ever bothering to step back and refactor. I've done that too, so it's not really a judgment of his coding in general, but it *is* difficult to deal with in this case. Now I need to though, because for starters his device reads are not const (if you know C++ then you should understand the ramifications of that) and they aren't easily made const because of the way it's implemented, so if I want to read pixels off the display device I can't do that, because htcw_gfx expects readable things to be able to const. I can fix that by moving all the driver communication code to a static template class whose arguments include the DC and CS pins, so I have statics per SPI device, but it means open heart surgery on Kurt's code, and I'm kind of overwhelmed trying to piece this all together. This is already my second attempt. :sigh: I love building things, but sometimes the act itself stifles the creative part. Like, the inspiration for the creation vs the act of creation itself - sometimes they are at odds. Hebrew has words for each part of that creative process. I wish English did.
To err is human. Fortune favors the monsters.
From a very quick scan the things that concern me with that code aren't whether reads are const or not. The number of "TODO", "BUG" and commented out lines imply that it hasn't been finished. May be suitable for what you need but having a clean codebase you can trust is far more important than whether it is fully optimised for language semantics.
-
From a very quick scan the things that concern me with that code aren't whether reads are const or not. The number of "TODO", "BUG" and commented out lines imply that it hasn't been finished. May be suitable for what you need but having a clean codebase you can trust is far more important than whether it is fully optimised for language semantics.
There's a bunch of that too. :~ I'd love a clean codebase, but this code does black magic with the ARM Cortex M7 SPI registers, and I don't know enough about it to write it from scratch.
To err is human. Fortune favors the monsters.
-
There's a bunch of that too. :~ I'd love a clean codebase, but this code does black magic with the ARM Cortex M7 SPI registers, and I don't know enough about it to write it from scratch.
To err is human. Fortune favors the monsters.
You could take the time to reverse engineer it. Then again, I remember a comment in somewhat similar code at a previous job: "Abandon hope, all ye who look at this code. Not even its author understands how it works, or why." :laugh:
Robust Services Core | Software Techniques for Lemmings | Articles
The fox knows many things, but the hedgehog knows one big thing. -
You could take the time to reverse engineer it. Then again, I remember a comment in somewhat similar code at a previous job: "Abandon hope, all ye who look at this code. Not even its author understands how it works, or why." :laugh:
Robust Services Core | Software Techniques for Lemmings | Articles
The fox knows many things, but the hedgehog knows one big thing.https://www.pjrc.com/teensy/K20P64M72SF1RM.pdf Starting at page 913 or so. Reading this nonsense is like reading sheet music to me - I can get it, if I squint, and I pray. This is what I'd be reverse engineering to get at. This is some of the code. It's all like this:
#elif defined(__IMXRT1052__) || defined(__IMXRT1062__) // Teensy 4.x
#define TCR_MASK \
(LPSPI_TCR_PCS(3) | LPSPI_TCR_FRAMESZ(31) | LPSPI_TCR_CONT | LPSPI_TCR_RXMSK)
void maybeUpdateTCR(
uint32_t requested_tcr_state) /*__attribute__((always_inline)) */ {
if ((_spi_tcr_current & TCR_MASK) != requested_tcr_state) {
bool dc_state_change = (_spi_tcr_current & LPSPI_TCR_PCS(3)) !=
(requested_tcr_state & LPSPI_TCR_PCS(3));
_spi_tcr_current = (_spi_tcr_current & ~TCR_MASK) | requested_tcr_state;
// only output when Transfer queue is empty.
if (!dc_state_change || !_dcpinmask) {
while ((_pimxrt_spi->FSR & 0x1f))
;
_pimxrt_spi->TCR = _spi_tcr_current; // update the TCR} else { waitTransmitComplete(); if (requested\_tcr\_state & LPSPI\_TCR\_PCS(3)) DIRECT\_WRITE\_HIGH(\_dcport, \_dcpinmask); else DIRECT\_WRITE\_LOW(\_dcport, \_dcpinmask); \_pimxrt\_spi->TCR = \_spi\_tcr\_current & ~(LPSPI\_TCR\_PCS(3) | LPSPI\_TCR\_CONT); // go ahead and update TCR anyway? } } } // BUGBUG:: currently assumming we only have CS\_0 as valid CS void writecommand\_cont(uint8\_t c) \_\_attribute\_\_((always\_inline)) { maybeUpdateTCR(\_tcr\_dc\_assert | LPSPI\_TCR\_FRAMESZ(7) /\*| LPSPI\_TCR\_CONT\*/); \_pimxrt\_spi->TDR = c; pending\_rx\_count++; // waitFifoNotFull(); } void writedata8\_cont(uint8\_t c) \_\_attribute\_\_((always\_inline)) { maybeUpdateTCR(\_tcr\_dc\_not\_assert | LPSPI\_TCR\_FRAMESZ(7) | LPSPI\_TCR\_CONT); \_pimxrt\_spi->TDR = c; pending\_rx\_count++; // waitFifoNotFull(); } void writedata16\_cont(uint16\_t d) \_\_attribute\_\_((always\_inline)) { maybeUpdateTCR(\_tcr\_dc\_not\_assert | LPSPI\_TCR\_FRAMESZ(15) | LPSPI\_TCR\_CONT); \_pimxrt\_spi->TDR = d; pending\_rx\_count++; // waitFifoNotFull(); }To err is human. Fortune favors the monsters.
-
I'm headdesking this code I'm trying to port. It's just a godawful mess. I'm barely treading water, even as I understand about 70% of it more or less, it's really difficult to modify. https://github.com/KurtE/ILI9341\_t3n specifically: https://github.com/KurtE/ILI9341\_t3n/blob/master/src/ILI9341\_t3n.cpp Kurt's a nice guy. I've corresponded with him, not about this code directly, but about my attempt at making similar code that works with htcw_gfx, my graphics library for IoT devices. That said, I hate the way he coded this. I feel like design was either an afterthought, or he just added to it over and over without ever bothering to step back and refactor. I've done that too, so it's not really a judgment of his coding in general, but it *is* difficult to deal with in this case. Now I need to though, because for starters his device reads are not const (if you know C++ then you should understand the ramifications of that) and they aren't easily made const because of the way it's implemented, so if I want to read pixels off the display device I can't do that, because htcw_gfx expects readable things to be able to const. I can fix that by moving all the driver communication code to a static template class whose arguments include the DC and CS pins, so I have statics per SPI device, but it means open heart surgery on Kurt's code, and I'm kind of overwhelmed trying to piece this all together. This is already my second attempt. :sigh: I love building things, but sometimes the act itself stifles the creative part. Like, the inspiration for the creation vs the act of creation itself - sometimes they are at odds. Hebrew has words for each part of that creative process. I wish English did.
To err is human. Fortune favors the monsters.
I think of someone else's code as clay; to be reworked if necessary; incrementally. Reworking code also helps me understand it better; even if it's just changing local variable names. But just staring at it ... not so much. Be one with the code.
"Before entering on an understanding, I have meditated for a long time, and have foreseen what might happen. It is not genius which reveals to me suddenly, secretly, what I have to say or to do in a circumstance unexpected by other people; it is reflection, it is meditation." - Napoleon I
-
I think of someone else's code as clay; to be reworked if necessary; incrementally. Reworking code also helps me understand it better; even if it's just changing local variable names. But just staring at it ... not so much. Be one with the code.
"Before entering on an understanding, I have meditated for a long time, and have foreseen what might happen. It is not genius which reveals to me suddenly, secretly, what I have to say or to do in a circumstance unexpected by other people; it is reflection, it is meditation." - Napoleon I
All I can say is you have not seen this code. :~
To err is human. Fortune favors the monsters.
-
I'm headdesking this code I'm trying to port. It's just a godawful mess. I'm barely treading water, even as I understand about 70% of it more or less, it's really difficult to modify. https://github.com/KurtE/ILI9341\_t3n specifically: https://github.com/KurtE/ILI9341\_t3n/blob/master/src/ILI9341\_t3n.cpp Kurt's a nice guy. I've corresponded with him, not about this code directly, but about my attempt at making similar code that works with htcw_gfx, my graphics library for IoT devices. That said, I hate the way he coded this. I feel like design was either an afterthought, or he just added to it over and over without ever bothering to step back and refactor. I've done that too, so it's not really a judgment of his coding in general, but it *is* difficult to deal with in this case. Now I need to though, because for starters his device reads are not const (if you know C++ then you should understand the ramifications of that) and they aren't easily made const because of the way it's implemented, so if I want to read pixels off the display device I can't do that, because htcw_gfx expects readable things to be able to const. I can fix that by moving all the driver communication code to a static template class whose arguments include the DC and CS pins, so I have statics per SPI device, but it means open heart surgery on Kurt's code, and I'm kind of overwhelmed trying to piece this all together. This is already my second attempt. :sigh: I love building things, but sometimes the act itself stifles the creative part. Like, the inspiration for the creation vs the act of creation itself - sometimes they are at odds. Hebrew has words for each part of that creative process. I wish English did.
To err is human. Fortune favors the monsters.
Reformat file to tab/space/bracket system that your mind comprehends best. Start with one variable. Figure out what it does. Universally rename it so you can understand it and not think about it again. Repeat x 1 billion or so. Continually perform incantations to the appropriate gods, including all the curse words you know, and make up some for good measure. Swear some more. Before you know it, success!!!
Our Forgotten Astronomy | Object Oriented Programming with C++
-
Reformat file to tab/space/bracket system that your mind comprehends best. Start with one variable. Figure out what it does. Universally rename it so you can understand it and not think about it again. Repeat x 1 billion or so. Continually perform incantations to the appropriate gods, including all the curse words you know, and make up some for good measure. Swear some more. Before you know it, success!!!
Our Forgotten Astronomy | Object Oriented Programming with C++
I can't rename the processor's SPI register constants. Let me show you what I'm working with. Frankly the variable names don't bother me. There aren't many variables, and I understand most of the class members like _csport and _dcport at this point. My main issue is the behavior of the registers. I have the manual but it reads like bad stereo instructions
#elif defined(__IMXRT1052__) || defined(__IMXRT1062__) // Teensy 4.x
#define TCR_MASK \
(LPSPI_TCR_PCS(3) | LPSPI_TCR_FRAMESZ(31) | LPSPI_TCR_CONT | LPSPI_TCR_RXMSK)
void maybeUpdateTCR(
uint32_t requested_tcr_state) /*__attribute__((always_inline)) */ {
if ((_spi_tcr_current & TCR_MASK) != requested_tcr_state) {
bool dc_state_change = (_spi_tcr_current & LPSPI_TCR_PCS(3)) !=
(requested_tcr_state & LPSPI_TCR_PCS(3));
_spi_tcr_current = (_spi_tcr_current & ~TCR_MASK) | requested_tcr_state;
// only output when Transfer queue is empty.
if (!dc_state_change || !_dcpinmask) {
while ((_pimxrt_spi->FSR & 0x1f))
;
_pimxrt_spi->TCR = _spi_tcr_current; // update the TCR} else { waitTransmitComplete(); if (requested\_tcr\_state & LPSPI\_TCR\_PCS(3)) DIRECT\_WRITE\_HIGH(\_dcport, \_dcpinmask); else DIRECT\_WRITE\_LOW(\_dcport, \_dcpinmask); \_pimxrt\_spi->TCR = \_spi\_tcr\_current & ~(LPSPI\_TCR\_PCS(3) | LPSPI\_TCR\_CONT); // go ahead and update TCR anyway? } } } // BUGBUG:: currently assumming we only have CS\_0 as valid CS void writecommand\_cont(uint8\_t c) \_\_attribute\_\_((always\_inline)) { maybeUpdateTCR(\_tcr\_dc\_assert | LPSPI\_TCR\_FRAMESZ(7) /\*| LPSPI\_TCR\_CONT\*/); \_pimxrt\_spi->TDR = c; pending\_rx\_count++; // waitFifoNotFull(); } void writedata8\_cont(uint8\_t c) \_\_attribute\_\_((always\_inline)) { maybeUpdateTCR(\_tcr\_dc\_not\_assert | LPSPI\_TCR\_FRAMESZ(7) | LPSPI\_TCR\_CONT); \_pimxrt\_spi->TDR = c; pending\_rx\_count++; // waitFifoNotFull(); } void writedata16\_cont(uint16\_t d) \_\_attribute\_\_((always\_inline)) { maybeUpdateTCR(\_tcr\_dc\_not\_assert | LPSPI\_TCR\_FRAMESZ(15) | LPSPI\_TCR\_CONT); \_pimxrt\_spi->TDR = d; pending\_rx\_count++; // waitFifoNotFull(); -
I'm headdesking this code I'm trying to port. It's just a godawful mess. I'm barely treading water, even as I understand about 70% of it more or less, it's really difficult to modify. https://github.com/KurtE/ILI9341\_t3n specifically: https://github.com/KurtE/ILI9341\_t3n/blob/master/src/ILI9341\_t3n.cpp Kurt's a nice guy. I've corresponded with him, not about this code directly, but about my attempt at making similar code that works with htcw_gfx, my graphics library for IoT devices. That said, I hate the way he coded this. I feel like design was either an afterthought, or he just added to it over and over without ever bothering to step back and refactor. I've done that too, so it's not really a judgment of his coding in general, but it *is* difficult to deal with in this case. Now I need to though, because for starters his device reads are not const (if you know C++ then you should understand the ramifications of that) and they aren't easily made const because of the way it's implemented, so if I want to read pixels off the display device I can't do that, because htcw_gfx expects readable things to be able to const. I can fix that by moving all the driver communication code to a static template class whose arguments include the DC and CS pins, so I have statics per SPI device, but it means open heart surgery on Kurt's code, and I'm kind of overwhelmed trying to piece this all together. This is already my second attempt. :sigh: I love building things, but sometimes the act itself stifles the creative part. Like, the inspiration for the creation vs the act of creation itself - sometimes they are at odds. Hebrew has words for each part of that creative process. I wish English did.
To err is human. Fortune favors the monsters.
Anarchy, Open source is wierd. I took a look at the link you posted. It's a fork of a library written by Paul Stoffregen[^] under the MIT license. Looking at that guys code reveals that it's yet another fork of the Adafruit library[^] with the BSD license removed. So it looks like: * Kurt (MIT licensed fork of PaulStoffregen) * PaulStoffregen (MIT licensed fork of Adafruit) * Adafruit original code is BSD licenced. It honestly feels like you'd need an army of lawyers to use alot of open source code. I've always avoided it.
-
Anarchy, Open source is wierd. I took a look at the link you posted. It's a fork of a library written by Paul Stoffregen[^] under the MIT license. Looking at that guys code reveals that it's yet another fork of the Adafruit library[^] with the BSD license removed. So it looks like: * Kurt (MIT licensed fork of PaulStoffregen) * PaulStoffregen (MIT licensed fork of Adafruit) * Adafruit original code is BSD licenced. It honestly feels like you'd need an army of lawyers to use alot of open source code. I've always avoided it.
I'm aware of where it came from, and I'm very familiar with the Adafruit rendition. The thing about Paul and Kurt's implementations is they are both optimized for the teensy, with Kurt's essentially being mostly the same as Paul's except with DMA. The teensy specific stuff is what I'm copying. I need nothing from Adafruit, so as far as I'm concerned what I'm copying is MIT, as is what I wrote. If Adafruit wants to come at me, they can sure try. Otherwise, I feel like this is fair use. Most of the IoT ecosystem is open source, so you either get used to it, or find another arena to develop in.
To err is human. Fortune favors the monsters.
-
I can't rename the processor's SPI register constants. Let me show you what I'm working with. Frankly the variable names don't bother me. There aren't many variables, and I understand most of the class members like _csport and _dcport at this point. My main issue is the behavior of the registers. I have the manual but it reads like bad stereo instructions
#elif defined(__IMXRT1052__) || defined(__IMXRT1062__) // Teensy 4.x
#define TCR_MASK \
(LPSPI_TCR_PCS(3) | LPSPI_TCR_FRAMESZ(31) | LPSPI_TCR_CONT | LPSPI_TCR_RXMSK)
void maybeUpdateTCR(
uint32_t requested_tcr_state) /*__attribute__((always_inline)) */ {
if ((_spi_tcr_current & TCR_MASK) != requested_tcr_state) {
bool dc_state_change = (_spi_tcr_current & LPSPI_TCR_PCS(3)) !=
(requested_tcr_state & LPSPI_TCR_PCS(3));
_spi_tcr_current = (_spi_tcr_current & ~TCR_MASK) | requested_tcr_state;
// only output when Transfer queue is empty.
if (!dc_state_change || !_dcpinmask) {
while ((_pimxrt_spi->FSR & 0x1f))
;
_pimxrt_spi->TCR = _spi_tcr_current; // update the TCR} else { waitTransmitComplete(); if (requested\_tcr\_state & LPSPI\_TCR\_PCS(3)) DIRECT\_WRITE\_HIGH(\_dcport, \_dcpinmask); else DIRECT\_WRITE\_LOW(\_dcport, \_dcpinmask); \_pimxrt\_spi->TCR = \_spi\_tcr\_current & ~(LPSPI\_TCR\_PCS(3) | LPSPI\_TCR\_CONT); // go ahead and update TCR anyway? } } } // BUGBUG:: currently assumming we only have CS\_0 as valid CS void writecommand\_cont(uint8\_t c) \_\_attribute\_\_((always\_inline)) { maybeUpdateTCR(\_tcr\_dc\_assert | LPSPI\_TCR\_FRAMESZ(7) /\*| LPSPI\_TCR\_CONT\*/); \_pimxrt\_spi->TDR = c; pending\_rx\_count++; // waitFifoNotFull(); } void writedata8\_cont(uint8\_t c) \_\_attribute\_\_((always\_inline)) { maybeUpdateTCR(\_tcr\_dc\_not\_assert | LPSPI\_TCR\_FRAMESZ(7) | LPSPI\_TCR\_CONT); \_pimxrt\_spi->TDR = c; pending\_rx\_count++; // waitFifoNotFull(); } void writedata16\_cont(uint16\_t d) \_\_attribute\_\_((always\_inline)) { maybeUpdateTCR(\_tcr\_dc\_not\_assert | LPSPI\_TCR\_FRAMESZ(15) | LPSPI\_TCR\_CONT); \_pimxrt\_spi->TDR = d; pending\_rx\_count++; // waitFifoNotFull();honey the codewitch wrote:
I can't rename the processor's SPI register constants.
The magic of C++ macros allows any possibility! Have fun swearing!
Our Forgotten Astronomy | Object Oriented Programming with C++
