Memory w25q1128jv spi #3
Conversation
|
|
@EricPedley @DanielHeEGG , I updated my drivers so its more readible and stuff. I also added a new Memory dump function and made the memory drivers just use a unspecified 64 byte array as an input and output instead of a struct |
| @@ -0,0 +1,186 @@ | |||
| /* | |||
There was a problem hiding this comment.
we dont need header blocks lol
There was a problem hiding this comment.
lol, idk why thats there. ill fix it
|
|
||
| #include <cmath> | ||
|
|
||
| #if defined(STM32F1) |
There was a problem hiding this comment.
let's verify compatibility on F4 series and add support
There was a problem hiding this comment.
good idea, will take it out until verified compatible.
| #include "stm32f1xx_hal.h" | ||
| #endif | ||
|
|
||
| #ifndef SPI_Timeout |
There was a problem hiding this comment.
As per talk in PR #4, use unique timeout for each driver. use correct naming convention, suggest add prefix. MEMORY_SPI_TIMEOUT and remove #ifndef block
There was a problem hiding this comment.
sounds good. Im guessing this is an older comment and we're gonna do a MEMORY_SPI_TIMEOUT as a passed in, private variable?
| enum class State { CHIP_ENABLE, PAGE_WRITE, COMPLETE, ERROR }; | ||
|
|
||
| /** | ||
| * @param hspi SPI bus handler |
There was a problem hiding this comment.
why is this line off and why did the formatter not fix it lol
| int ChipReadDump(uint8_t (&chipData)[64]); | ||
|
|
||
| /* 64 byte structs for use in UCIRP projects*/ | ||
| #pragma pack(push, 1) |
There was a problem hiding this comment.
the driver doesn't need to / should not care what packet its storing
There was a problem hiding this comment.
Agreed, none of the function in my code use these packets.
I left the packets in here so that whenever someone uses our memory module, the packets come imported with the memory drivers. I can take it out if you want, but I like the idea that other people who use the memory drivers can simply just define their needed packet without needing to define the struct in their main.
| return 0; | ||
| } | ||
|
|
||
| MemoryW25q1128jvSpi::State MemoryW25q1128jvSpi::ChipWrite(uint8_t (&data)[64]) { |
There was a problem hiding this comment.
this uint8_t (&data)[64] is uhhhh, unusual? i've never seen this before, uint8_t *data is fine.
is this some fancy c++ thing im not aware of?
There was a problem hiding this comment.
Yea, I looked up this one as a solution. uint8_t *data is way more standard to see, but I just wanted to specify that the array to be sent into the function has to be exactly 64 elements. We can probably change the code to uint8_t *data if you don't want to the end user to be errored out by the compiler when they try to send an array of different size than 64 byes, but personally, I like this implementation.
|
|
||
| // if address variable goes over the amount of pages memory has | ||
| // no more collected data will be written into the memory | ||
| if (address > 0xFFFFFF) _state = State::ERROR; |
There was a problem hiding this comment.
i think just _state = ERROR; is fine, but we can discuss this further
There was a problem hiding this comment.
yea, I remember looking at your altimeter drivers and seeing you doing the _state = ERROR;. I honestly don't know why the compiler isn't telling you that the syntax is an error because it was for me. I asked Eric about it, can't remember what he said but he's also confused why yours works
| * @retval Operation Sucess, 0 for success | ||
| * @retval Operation Failure, -1 for SPI protocol failure | ||
| */ | ||
| int ChipRead(uint32_t read_address, uint8_t (&chipData)[64]); |
| return 0; | ||
| } | ||
|
|
||
| int MemoryW25q1128jvSpi::ChipReadDump(uint8_t (&chipData)[64]) { |
There was a problem hiding this comment.
im confused by this method, it always just reads block 0?
There was a problem hiding this comment.
static uint32_t dumpAddress = 0; basically acts like a private variable in this function. So, its value is initialised as 0 once and then in the next call to this function, the variable's value from the last function call is remembered while the variable stays in the scope of the function. I can assure you that it does increment properly for every function call.
Working memory drivers forw25q1128jv AFS 2.1