Reading a 16-bit word via SPI [STM32]

[SiliconWizard]

SPI clock rate is irrelevant as long as it doesn't exceed the max clock rate for the device, so 13MHz should be fine if you don't need any faster. I don't know what your system clock is, and even less so that of the corresponding APB, but your SPI clock can only be a fraction of it (by powers of two prescaler), so obviously you won't be able to generate just any frequency you want. You can adjust the APB clock though, doesn't have to be the same as the CPU clock.

As to the transfer, again your scope captures help figuring out what's wrong.

Now you can see there are actually TWO 16-bit transfers per call, which is not what you intended. (It will even actually overwrite things on the stack, which is bad. Here is why.)

I had to take a look at the HAL source code for HAL_SPI_Receive() to understand (maybe it's documented somewhere, but looking at the code was just much faster).
When the SPI peripheral is configured for 16-bit transfers, the Size parameter is actually NOT the size in bytes, but the number of 16-bit transfers.

Yeah, ST developers are nice like that, and produce consistent interfaces (ahem.) The fact that the parameter which is a pointer to the buffer is of type "uint8_t *" is completely misleading; it kind of makes you assume the Size parameter would naturally be the size in bytes. But nope. They should have declared the buffer param as "void *" instead IMO to make it less ambiguous (and it would not have required any annoying cast...), and they should have called the Size parameter something else. Oh well...

So in your case, you should pass ONLY 1 as the Size parameter, and a pointer to a uint16_t variable (like you did first), because the function actually writes the received data by 16-bit chunks (not sure if Cortex-M cores like unaligned accesses, but I don't really like them myself anyway.)

Passing 2 as the Size parameter, as you did, will issue TWO 16-bit transfers (which you can see on your scope). The second one will write somewhere on the stack that may have nasty consequences (basically you're unwillingly having a buffer overflow here.)

[A.Ersoz]

I modified a little bit of my code. Now there are 16 clock cycles. I have attached the output result in Keil debug session. The values are correct but for just make sure I want to see all the amplitude values of the signal. So, how can I adjust digitized time period, acquisition time, and plot the digitized signal?

Code: [Select]

uint8_t ADC_Buf[2];
uint16_t Sample;

HAL_GPIO_WritePin(ADC_CS_GPIO_Port, ADC_CS_Pin, GPIO_PIN_RESET);
HAL_SPI_Receive(&hspi4,ADC_Buf,1,100);
Sample = (((uint16_t) ADC_Buf[0]) << 8 | ADC_Buf[1]) & 0x0FFF;
volt = (float)(Sample * (5.0 / 4096.0)); //
testdata_in[i++]=volt;
i %= 16;
HAL_GPIO_WritePin(ADC_CS_GPIO_Port, ADC_CS_Pin, GPIO_PIN_SET);

[SiliconWizard]

Sampling time can be set using HAL_ADC_ConfigChannel() (from a ADC_ChannelConfTypeDef parameter)
For the sampling rate, you can set a timer trigger for the ADC.
You can then fill in a buffer of samples, and then read/process the whole buffer once the acquisition is done.

[JustMeHere]

People will piss and moan, but I use a union to do this.

Something like
union
{
   uint8_t data[2];
   uint16_t result;
}

I know people will give reasons not to do this, but it works.  The objection is "type pruning" but if it works, saves cycles, and saves memory; then in low level C why not?  I'm not looking for portability.

[A.Ersoz]

I modified my code for interrupt mode:

Main Code:

Code: [Select]


void ADC_Conversion_of_Voltage_Transients()
{

HAL_SPI_Receive(&hspi4,ADC_Buf,16,100);
Sample = (((uint16_t) ADC_Buf[1]) << 8 | ADC_Buf[0]) & 0x0FFF;
volt = (float)(Sample * (5.0 / 4096.0)); //
testdata_in[i++]=volt;
i %= 16;

}




Intterrupt handler function:

Code: [Select]


void TIM4_IRQHandler(void)
{
  /* USER CODE BEGIN TIM4_IRQn 0 */

  /* USER CODE END TIM4_IRQn 0 */
  HAL_TIM_IRQHandler(&htim4);
  /* USER CODE BEGIN TIM4_IRQn 1 */
HAL_GPIO_TogglePin(ADC_CS_GPIO_Port, ADC_CS_Pin);
Timer_Flag=1;

  /* USER CODE END TIM4_IRQn 1 */
}



Call function:

Code: [Select]


if (Timer_Flag)
{
ADC_Conversion_of_Voltage_Transients();
Timer_Flag=0;
}


But the problem is ADC SCK is continous without realted CS position which is controlled timer.

Result. ADC SCK (green plot) and ADC CS (yellow plot).

I will try 'union' function.

[SiliconWizard]

Ignore my last post, I was suddenly thinking about the internal ADC for some reason.

Regarding your IRQ handler, I think you're on the right track, but you'll have to think it through a little bit more.
You're toggling /CS each time the interrupt fires, and setting a flag to start the SPI transfer outside of the IRQ. This isn't right, as this will trigger an SPI transfer when /CS is low, but ALSO when /CS is high!

Think about it...

[A.Ersoz]

Is it like this?

Code: [Select]


void TIM4_IRQHandler(void)
{
  /* USER CODE BEGIN TIM4_IRQn 0 */

  /* USER CODE END TIM4_IRQn 0 */
  HAL_TIM_IRQHandler(&htim4);
  /* USER CODE BEGIN TIM4_IRQn 1 */
HAL_GPIO_WritePin(ADC_CS_GPIO_Port, ADC_CS_Pin, GPIO_PIN_RESET);
Timer_Flag=1;
HAL_GPIO_WritePin(ADC_CS_GPIO_Port, ADC_CS_Pin, GPIO_PIN_SET);



  /* USER CODE END TIM4_IRQn 1 */
}




Code: [Select]

void ADC_Conversion_of_Voltage_Transients()
{
Timer_Flag=1;
HAL_SPI_Receive(&hspi4,ADC_Buf,16,100);
Sample = (((uint16_t) ADC_Buf[1]) << 8 | ADC_Buf[0]) & 0x0FFF;
volt = (float)(Sample * (5.0 / 4096.0)); //
testdata_in[i++]=volt;
i %= 16;
Timer_Flag=0;

}


[SiliconWizard]

Not yet.
In this case you're making ultra short /CS pulses.

Set /CS low in your IRQ handler as you already do. (This makes sense to do this here as this is what starts the conversion cycle.)

Then set /CS high at the end of your ADC_Conversion_of_Voltage_Transients() function only.

[A.Ersoz]

Thank you for your help! But something I miss. ADC CS keep high every time. Then, number of clock cycles are true but between of the each cycles has distortion.

Code: [Select]


void TIM4_IRQHandler(void)
{
  /* USER CODE BEGIN TIM4_IRQn 0 */

  /* USER CODE END TIM4_IRQn 0 */
  HAL_TIM_IRQHandler(&htim4);
  /* USER CODE BEGIN TIM4_IRQn 1 */
HAL_GPIO_WritePin(ADC_CS_GPIO_Port, ADC_CS_Pin, GPIO_PIN_RESET);

  /* USER CODE END TIM4_IRQn 1 */
}



Code: [Select]

void ADC_Conversion_of_Voltage_Transients()
{
HAL_SPI_Receive(&hspi4,ADC_Buf,16,100);
Sample = (((uint16_t) ADC_Buf[1]) << 8 | ADC_Buf[0]) & 0x0FFF;
volt = (float)(Sample * (5.0 / 4096.0)); //
testdata_in[i++]=volt;
i %= 16;
HAL_GPIO_WritePin(ADC_CS_GPIO_Port, ADC_CS_Pin, GPIO_PIN_SET);
}



Code: [Select]


while(1)
{
ADC_Conversion_of_Voltage_Transients();
}


[SiliconWizard]

This time you removed the "flag" mechanism, making this unsynchronized...
You still need the flag.

(You could also issue the whole transfer from the IRQ handler, but it's usually not recommended to spend too much time inside IRQ handlers, so the flag mechanism is better.)

Last note: don't forget to declare the flag global variable with a "volatile" qualifier; otherwise, it may not work as expected with compiler optimizations. This is a very common issue and has been discussed here many times.

[A.Ersoz]

Actually, I didn't get SCK, CS, and SDO signals. I defined the Timer_Flag as a global variable and connect to the main function with 'extern'.

Code: [Select]


void TIM4_IRQHandler(void)
{
  /* USER CODE BEGIN TIM4_IRQn 0 */

  /* USER CODE END TIM4_IRQn 0 */
  HAL_TIM_IRQHandler(&htim4);
  /* USER CODE BEGIN TIM4_IRQn 1 */
HAL_GPIO_WritePin(ADC_CS_GPIO_Port, ADC_CS_Pin, GPIO_PIN_RESET);
Timer_Flag=1;

  /* USER CODE END TIM4_IRQn 1 */
}



Code: [Select]

void ADC_Conversion_of_Voltage_Transients()
{
        HAL_SPI_Receive(&hspi4,ADC_Buf,2,100);
HAL_GPIO_WritePin(ADC_CS_GPIO_Port, ADC_CS_Pin, GPIO_PIN_SET);
Sample = (((uint16_t) ADC_Buf[1]) << 8 | ADC_Buf[0]) & 0x0FFF;
volt = (float)(Sample * (5.0 / 4096.0)); //
testdata_in[i++]=volt;
i %= 16;

}



Code: [Select]

                if(Timer_Flag)
{
ADC_Conversion_of_Voltage_Transients();
Timer_Flag=0;
}