Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Serial.availableForWrite() and Serial.blockOnOverrun() implementation, USB Serial fixes for STM32F2 #812

Merged
merged 10 commits into from
Mar 10, 2016
Merged
4 changes: 4 additions & 0 deletions hal/inc/hal_dynalib_usart.h
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,10 @@ DYNALIB_FN(hal_usart,HAL_USART_Peek_Data)
DYNALIB_FN(hal_usart,HAL_USART_Flush_Data)
DYNALIB_FN(hal_usart,HAL_USART_Is_Enabled)
DYNALIB_FN(hal_usart,HAL_USART_Half_Duplex)
DYNALIB_FN(hal_usart,HAL_USART_Available_Data_For_Write)
#ifdef USB_CDC_ENABLE
DYNALIB_FN(hal_usart,USB_USART_Available_Data_For_Write)
#endif

DYNALIB_END(hal_usart)

Expand Down
1 change: 1 addition & 0 deletions hal/inc/usart_hal.h
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,7 @@ void HAL_USART_Init(HAL_USART_Serial serial, Ring_Buffer *rx_buffer, Ring_Buffer
void HAL_USART_Begin(HAL_USART_Serial serial, uint32_t baud);
void HAL_USART_End(HAL_USART_Serial serial);
uint32_t HAL_USART_Write_Data(HAL_USART_Serial serial, uint8_t data);
int32_t HAL_USART_Available_Data_For_Write(HAL_USART_Serial serial);
int32_t HAL_USART_Available_Data(HAL_USART_Serial serial);
int32_t HAL_USART_Read_Data(HAL_USART_Serial serial);
int32_t HAL_USART_Peek_Data(HAL_USART_Serial serial);
Expand Down
7 changes: 7 additions & 0 deletions hal/inc/usb_hal.h
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,12 @@ void USB_USART_LineCoding_BitRate_Handler(void (*handler)(uint32_t bitRate));
*/
uint8_t USB_USART_Available_Data(void);

/**
* Retrieves the number of bytes of data available in the TX buffer.
* @return
*/
int32_t USB_USART_Available_Data_For_Write(void);

/**
* Reads data from the input buffer.
* @param peek If the data should be peeked reather than fetched.
Expand All @@ -93,6 +99,7 @@ int32_t USB_USART_Receive_Data(uint8_t peek);
/**
* Sends data to the USB serial.
* @param Data The data to write.
* @return
*/
void USB_USART_Send_Data(uint8_t Data);
#endif
Expand Down
5 changes: 5 additions & 0 deletions hal/src/core/usart_hal.c
Original file line number Diff line number Diff line change
Expand Up @@ -218,6 +218,11 @@ void HAL_USART_End(HAL_USART_Serial serial)
usartMap[serial]->usart_transmitting = false;
}

int32_t HAL_USART_Available_Data_For_Write(HAL_USART_Serial serial)
{
return (unsigned int)(SERIAL_BUFFER_SIZE + usartMap[serial]->usart_tx_buffer->head - usartMap[serial]->usart_tx_buffer->tail) % SERIAL_BUFFER_SIZE;
}

uint32_t HAL_USART_Write_Data(HAL_USART_Serial serial, uint8_t data)
{
// interrupts are off and data in queue;
Expand Down
14 changes: 14 additions & 0 deletions hal/src/core/usb_hal.c
Original file line number Diff line number Diff line change
Expand Up @@ -197,6 +197,20 @@ int32_t USB_USART_Receive_Data(uint8_t peek)
return -1;
}

/*******************************************************************************
* Function Name : USB_USART_Available_Data_For_Write.
* Description : Return the length of available space in TX buffer
* Input : None.
* Return : Length.
*******************************************************************************/
int32_t USB_USART_Available_Data_For_Write(void)
{
if(bDeviceState == CONFIGURED)
return (USART_RX_DATA_SIZE - USART_Rx_ptr_in) % USART_RX_DATA_SIZE;
return -1;
}


/*******************************************************************************
* Function Name : USB_USART_Send_Data.
* Description : Send Data from USB_USART to USB Host.
Expand Down
9 changes: 9 additions & 0 deletions hal/src/gcc/usart_hal.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ struct Usart {
virtual void begin(uint32_t baud)=0;
virtual void end()=0;
virtual int32_t available()=0;
virtual int32_t availableForWrite()=0;
virtual int32_t read()=0;
virtual int32_t peek()=0;
virtual uint32_t write(uint8_t byte)=0;
Expand Down Expand Up @@ -84,6 +85,9 @@ class SocketUsartBase : public Usart
fillFromSocketIfNeeded();
return (rx->head-rx->tail) % SERIAL_BUFFER_SIZE;
}
virtual int32_t availableForWrite() override {
return (SERIAL_BUFFER_SIZE + tx->head - tx->tail) % SERIAL_BUFFER_SIZE;
}
virtual int32_t read() override {
fillFromSocketIfNeeded();
return read_char();
Expand Down Expand Up @@ -189,6 +193,11 @@ void HAL_USART_End(HAL_USART_Serial serial)
//usartMap(serial).end();
}

int32_t HAL_USART_Available_Data_For_Write(HAL_USART_Serial serial)
{
return usartMap(serial).availableForWrite();
}

uint32_t HAL_USART_Write_Data(HAL_USART_Serial serial, uint8_t data)
{
return usartMap(serial).write(data);
Expand Down
11 changes: 11 additions & 0 deletions hal/src/gcc/usb_hal.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,17 @@ int32_t USB_USART_Receive_Data(uint8_t peek)
return result;
}

/*******************************************************************************
* Function Name : USB_USART_Available_Data_For_Write.
* Description : Return the length of available space in TX buffer
* Input : None.
* Return : Length.
*******************************************************************************/
int32_t USB_USART_Available_Data_For_Write(void)
{
return 1;
}

/*******************************************************************************
* Function Name : USB_USART_Send_Data.
* Description : Send Data from USB_USART to USB Host.
Expand Down
6 changes: 6 additions & 0 deletions hal/src/stm32f2xx/usart_hal.c
Original file line number Diff line number Diff line change
Expand Up @@ -311,6 +311,12 @@ int32_t HAL_USART_Available_Data(HAL_USART_Serial serial)
return (unsigned int)(SERIAL_BUFFER_SIZE + usartMap[serial]->usart_rx_buffer->head - usartMap[serial]->usart_rx_buffer->tail) % SERIAL_BUFFER_SIZE;
}

int32_t HAL_USART_Available_Data_For_Write(HAL_USART_Serial serial)
{
return (unsigned int)(SERIAL_BUFFER_SIZE + usartMap[serial]->usart_tx_buffer->head - usartMap[serial]->usart_tx_buffer->tail) % SERIAL_BUFFER_SIZE;
}


int32_t HAL_USART_Read_Data(HAL_USART_Serial serial)
{
// if the head isn't ahead of the tail, we don't have any characters
Expand Down
80 changes: 54 additions & 26 deletions hal/src/stm32f2xx/usb_hal.c
Original file line number Diff line number Diff line change
Expand Up @@ -54,10 +54,13 @@ extern uint32_t USBD_OTG_EP1OUT_ISR_Handler(USB_OTG_CORE_HANDLE *pdev);
extern volatile LINE_CODING linecoding;
extern volatile uint8_t USB_DEVICE_CONFIGURED;
extern volatile uint8_t USB_Rx_Buffer[];
extern volatile uint8_t APP_Rx_Buffer[];
extern volatile uint32_t APP_Rx_ptr_in;
extern volatile uint16_t USB_Rx_length;
extern volatile uint16_t USB_Rx_ptr;
extern volatile uint32_t USB_Rx_Buffer_head;
extern volatile uint32_t USB_Rx_Buffer_tail;
extern volatile uint32_t USB_Rx_Buffer_length;
extern volatile uint8_t USB_Tx_Buffer[];
extern volatile uint32_t USB_Tx_Buffer_head;
extern volatile uint32_t USB_Tx_Buffer_tail;
extern volatile uint32_t USB_Tx_Buffer_length;
extern volatile uint8_t USB_Tx_State;
extern volatile uint8_t USB_Rx_State;
#endif
Expand Down Expand Up @@ -140,6 +143,10 @@ void USB_USART_LineCoding_BitRate_Handler(void (*handler)(uint32_t bitRate))
SetLineCodingBitRateHandler(handler);
}

static inline bool USB_USART_Connected() {
return linecoding.bitrate > 0 && USB_OTG_dev.dev.device_status == USB_OTG_CONFIGURED;
}

/*******************************************************************************
* Function Name : USB_USART_Available_Data.
* Description : Return the length of available data received from USB.
Expand All @@ -148,9 +155,14 @@ void USB_USART_LineCoding_BitRate_Handler(void (*handler)(uint32_t bitRate))
*******************************************************************************/
uint8_t USB_USART_Available_Data(void)
{
if(USB_Rx_State == 1)
if (USB_USART_Connected())
{
return (USB_Rx_length - USB_Rx_ptr);
int32_t available = 0;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm concerned that USB_Rx_Buffer_head / USB_Rx_Buffer_tail will be changed by the USB IRQ after being tested.

As a minimum, capture the tail and head into a local variables, and use that so we are certain they are consistent.

A context switch after comparing head >= tail could see head roll over to the start of the buffer, which would then result in the incorrect size being returned.

if (USB_Rx_Buffer_head >= USB_Rx_Buffer_tail)
available = USB_Rx_Buffer_head - USB_Rx_Buffer_tail;
else
available = USB_Rx_Buffer_length + USB_Rx_Buffer_head - USB_Rx_Buffer_tail;
return available;
}

return 0;
Expand All @@ -164,20 +176,34 @@ uint8_t USB_USART_Available_Data(void)
*******************************************************************************/
int32_t USB_USART_Receive_Data(uint8_t peek)
{
if(USB_Rx_State == 1)
if (USB_USART_Available_Data() > 0)
{
if(!peek && ((USB_Rx_length - USB_Rx_ptr) == 1))
{
USB_Rx_State = 0;

/* Prepare Out endpoint to receive next packet */
DCD_EP_PrepareRx(&USB_OTG_dev,
CDC_OUT_EP,
(uint8_t*)(USB_Rx_Buffer),
CDC_DATA_OUT_PACKET_SIZE);
uint32_t tail = USB_Rx_Buffer_tail;
uint8_t data = USB_Rx_Buffer[USB_Rx_Buffer_tail];
if (!peek) {
tail++;
if (tail == USB_Rx_Buffer_length)
tail = 0;
USB_Rx_Buffer_tail = tail;
}
return data;
}

return USB_Rx_Buffer[peek ? USB_Rx_ptr : USB_Rx_ptr++];
return -1;
}

/*******************************************************************************
* Function Name : USB_USART_Available_Data_For_Write.
* Description : Return the length of available space in TX buffer
* Input : None.
* Return : Length.
*******************************************************************************/
int32_t USB_USART_Available_Data_For_Write(void)
{
if (USB_USART_Connected())
{
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Similarly here - because USB_Tx_Buffer_tail is updated by the USB IRQ, it's best to capture that as a local variable first, then use it for comparison and computation to avoid an inconsistent computation.

return USB_TX_BUFFER_SIZE - (USB_Tx_Buffer_head >= USB_Tx_Buffer_tail ?
USB_Tx_Buffer_head - USB_Tx_Buffer_tail : USB_TX_BUFFER_SIZE + USB_Tx_Buffer_head - USB_Tx_Buffer_tail) - 1;
}

return -1;
Expand All @@ -191,18 +217,20 @@ int32_t USB_USART_Receive_Data(uint8_t peek)
*******************************************************************************/
void USB_USART_Send_Data(uint8_t Data)
{
APP_Rx_Buffer[APP_Rx_ptr_in] = Data;
int32_t available = 0;
do {
available = USB_USART_Available_Data_For_Write();
}
while (available < 1 && available != -1);
// Confirm once again that the Host is connected
if (USB_USART_Connected())
{
uint32_t head = USB_Tx_Buffer_head;

APP_Rx_ptr_in++;
USB_Tx_Buffer[USB_Tx_Buffer_head] = Data;

/* To avoid buffer overflow */
if(APP_Rx_ptr_in == APP_RX_DATA_SIZE)
{
APP_Rx_ptr_in = 0;
USB_Tx_Buffer_head = ++head % USB_TX_BUFFER_SIZE;
}

//Delay 100us to avoid losing the data
HAL_Delay_Microseconds(100);
}
#endif

Expand Down
11 changes: 11 additions & 0 deletions hal/src/template/usb_hal.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,17 @@ int32_t USB_USART_Receive_Data(uint8_t peek)
return -1;
}

/*******************************************************************************
* Function Name : USB_USART_Available_Data_For_Write.
* Description : Return the length of available space in TX buffer
* Input : None.
* Return : Length.
*******************************************************************************/
int32_t USB_USART_Available_Data_For_Write(void)
{
return 0;
}

/*******************************************************************************
* Function Name : USB_USART_Send_Data.
* Description : Send Data from USB_USART to USB Host.
Expand Down
5 changes: 3 additions & 2 deletions platform/MCU/STM32F2xx/SPARK_Firmware_Driver/inc/usbd_conf.h
Original file line number Diff line number Diff line change
Expand Up @@ -85,9 +85,10 @@
#define CDC_DATA_MAX_PACKET_SIZE 64 /* Endpoint IN & OUT Packet size */
#define CDC_CMD_PACKET_SZE 8 /* Control Endpoint Packet size */

#define CDC_IN_FRAME_INTERVAL 5 /* Number of micro-frames between IN transfers */
#define APP_RX_DATA_SIZE 256 /* Total size of IN buffer:
#define CDC_IN_FRAME_INTERVAL 1 /* Number of micro-frames between IN transfers */
#define USB_TX_BUFFER_SIZE 128 /* Total size of IN buffer:
APP_RX_DATA_SIZE*8/MAX_BAUDARATE*1000 should be > CDC_IN_FRAME_INTERVAL */
#define USB_RX_BUFFER_SIZE 128

#define APP_FOPS APP_fops

Expand Down
Loading