feat(mcal): implement SpiMaster peripheral for stm32f, lnx, and cli#539
feat(mcal): implement SpiMaster peripheral for stm32f, lnx, and cli#539priyaltaneja wants to merge 2 commits intomainfrom
Conversation
priyaltaneja
requested review from
BlakeFreer,
CameronBeneteau,
TylerStAmour,
langei and
samparent97
as code owners
Add SpiMaster implementations across all three platforms: - stm32f: HAL-based implementation with GPIO chip select control - lnx: spidev interface with configurable mode and speed - cli: interactive hex I/O for testing and simulation Includes Init()/Setup() functions for future bindings integration.
BlakeFreer
force-pushed
the
feature/spi-implementation
branch
from
77ec816 to
610a802
Compare
| } | ||
|
|
||
| // initialize chip select pin as output, set high (inactive) | ||
| GPIO_InitTypeDef GPIO_InitStruct = {0}; |
There was a problem hiding this comment.
glad you learned stm HAL pin initialization, but you don't need to configure the chip select pin manually. we will configure in in CubeMX so you can assume the Port/Pin is is initialized already
| /// initialize the SPI peripheral | ||
| /// must be called before using any transmission methods | ||
| /// typically called from bindings::Initialize() | ||
| void Init() { |
There was a problem hiding this comment.
I like the Init() function! it's a good idea since we want to guarantee that the CS pin is high on startup
I've seen the if(initialized_) and if(!initialized_) pattern used before, but I find it usually only clutters the code and doesn't provide any value. In Init() there is no harm in initializing twice (the outcome is the same), and in Transmit() / Receive() / TransmitReceive(), the transmission won't happen until you separately edit the code to call Init()
We don't need to protect the developer from forgetting to call Init(), so can you remove the initialized_ field and if(initialized_ checks? Similarly, we don't need to check that hspi != null
| } | ||
|
|
||
| /// check if the spi peripheral has been initialized | ||
| bool IsInitialized() const { |
There was a problem hiding this comment.
don't need this, see comment above
| } | ||
|
|
||
| /// get the underlying hal spi handle | ||
| SPI_HandleTypeDef* GetHandle() const { |
There was a problem hiding this comment.
I don't think we need this. When would it be called?
| /// initialize the spi device | ||
| /// opens the device file and configures spi parameters | ||
| void Setup() { | ||
| if (initialized_) { |
There was a problem hiding this comment.
see this comment https://github.com/macformula/racecar/pull/539/files#r2528588442, don't need the initialized field & checks
3 participants
Add SpiMaster implementations across all three platforms:
Includes Init()/Setup() functions for future bindings integration.