Work on Devices.Spi #428
Work on Devices.Spi #428
Conversation
- update code to latest version of the managed class library - move gpioPort[] to targetPAL to allow reuse - work on NativeTransfer for byte[] transfers (code improvement, add comments) Signed-off-by: José Simões <jose.simoes@eclo.solutions>
josesimoes
added
Series: STM32xx
|
Hi @josesimoes, I'm nanoFramework bot. A human will be reviewing it shortly. 😉 |
| @@ -273,22 +265,22 @@ HRESULT Library_win_dev_spi_native_Windows_Devices_Spi_SpiDevice::GetDeviceSelec | |||
|
|
|||
| char* retVal = ""; | |||
| #if STM32_SPI_USE_SPI1 | |||
| strcat(retVal,"SPI1"); | |||
| strcat(retVal,"SPI1,"); | |||
There was a problem hiding this comment.
I do not agree with this change.
What if the config is only using SPI1 ? You will end up with "SPI1," as a result string, not "SPI1".
Same for the following changes on other SPIx, of course.
There was a problem hiding this comment.
If you are splitting this string in managed code it will work OK.
Consider that you have SPI2 and SPI5 enable for example.
With the previous approach it would end up on ",SPI2,SPI5" now it's "SPI2,SPI5,"
Ideally we would add the "," only as required but that would mean adding a lot of code to handle just this.
There was a problem hiding this comment.
I understand.
A quick and dirty fix, for both versions of code, could then be to remove the first/last char of the resulting string if it is a "," ?
There was a problem hiding this comment.
Yes, that would work! And it just requires a couple of lines. 👍
Care to add that to your incoming PR? 😉
There was a problem hiding this comment.
Ok. I will do that. Which version shall I use ? I would not like to remove the wrong char :D
There was a problem hiding this comment.
Feel free to choose the one that works best for you.
In the end it will end up on a nice and properly formatted string. 😉
There was a problem hiding this comment.
I will take yours, because the last char of the resulting string will always be a comma ;) No need to think more than "remove the last char of that string" ;)
I will only add that comma on the "SPI6" string.
There was a problem hiding this comment.
strcat(retval, retval = "" ? "SPI1" : ", SPI1") does this compute ?
There was a problem hiding this comment.
It does, but... Won't that require an extra IF clause on each string?
There was a problem hiding this comment.
@piwi1263 : Systematically removing the last char will be faster and shorter, I think ;) No need to think : remove !
Signed-off-by: José Simões jose.simoes@eclo.solutions