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

Created a UART API definition #17

Open
wants to merge 1 commit into
base: linux-descriptors
Choose a base branch
from
Open
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
98 changes: 98 additions & 0 deletions include/uart/api.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,98 @@
/** \file include/uart/api.h
* This file is for defining a common API for accessing UARTs.
**/

#ifndef UART_API_H
#define UART_API_H

#include "fx2types.h"
#include "stdarg.h"

/**
* \brief Initalizes UART.
* Returns TRUE if initialization is successful.
* \param Rate See uartX_set_baud().
**/
BOOL uartX_init(enum uart_baud rate, ...);

/**
Copy link
Owner

Choose a reason for hiding this comment

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

This documentation of this enum still hasn't been fixed.

Copy link
Author

Choose a reason for hiding this comment

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

Not really sure what kind of a doc you want. I just added 2 more line saying less than 0 not valid, greater than 0 valid. Is there something else you want me to add?

Copy link
Owner

Choose a reason for hiding this comment

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

Take a look at other examples in the code. Remember consistency is important!

From fx2int.h

/**
 * \brief Interrupt numbers for standard FX2 interrupts.
 **/
typedef enum {
 IE0_ISR=0, ///< External interrupt 0
 TF0_ISR, ///< Timer 0 interrupt
 IE1_ISR, ///< External interrupt 1
 TF1_ISR, ///< Timer 1 interrupt  
 TI_0_ISR, ///< Serial port 0 transmit or receive interrupt
 TF2_ISR, ///< Timer 2 interrupt
 RESUME_ISR, ///< Resume interrupt
 TI_1_ISR, ///< Serial port 1 transmit or receive interrupt
 USBINT_ISR, ///< USB Interrupt.  An interrupt handler for this should only be used if not using auto vectored interrupts with INT2
 I2CINT_ISR, ///< I2C Bus interrupt
 IE4_ISR, ///< External interrupt 4.  An interrupt handler for this should only be used if not using auto vectored interrupts with INT4
 IE5_ISR, ///< External interrupt 5
 IE6_ISR, ///< External interrupt 6
 // Better names for the USART interrupts
 USART0_ISR = TI_0_ISR, ///< Better name for USART0 interrupt
 USART1_ISR = TI_1_ISR, ///< Better name for USART1 interrupt
} FX2_ISR;

From fx2macros.h

/**
 * \brief Used for getting and setting the CPU clock speed.
 **/
typedef enum { CLK_12M =0, CLK_24M, CLK_48M} CLK_SPD;

Everything else has a \brief for the short description. You haven't explained what the BAUD_ANY and BAUD_FASTEST should do.

This should probably be a typedef too.

Copy link
Author

Choose a reason for hiding this comment

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

I generally prefer not to use typedef. I prefer doing enum name. This is part of the Linux coding guidelines given https://www.kernel.org/doc/Documentation/CodingStyle , Chapter 5.

Copy link
Author

Choose a reason for hiding this comment

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

Fixed

Copy link
Owner

Choose a reason for hiding this comment

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

Please use the typedefs. This isn't the Linux kernel -- please use style consistent with the rest of the code.

Copy link
Author

Choose a reason for hiding this comment

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

Fixed.

* \brief enum Standard available baud rates.
* BAUD_FASTEST will give you the fastest baud rate the UART can operate at. WARNING:
* This is allowed to be a non-standard baud rate.
* Enum values greater than 0 are supported depending on the UART in use.To check
Copy link
Owner

Choose a reason for hiding this comment

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

Space after full stop.

* if the baud is supported call uartX_set_baud and use the return value to determine
* if the mode is supported.
**/
typedef enum
{
BAUD_FASTEST = -2, ///< Fastest baud rate the UART can operate at. WARNING:This is allowed to be a non-standard baud rate.
BAUD_ANY = -1, ///< Virtual baud, Most efficient BAUD rate for the UART.
BAUD_INVALID = 0, ///< A divider between actual baud rates and virtual baud rates.
BAUD_2400 = 1, ///< Sets the baud rate to 2400.
BAUD_4800 = 2, ///< Sets the baud rate to 4800.
BAUD_9600 = 3, ///< Sets the baud rate to 9600.
BAUD_19200 = 4, ///< Sets the baud rate to 19200.
BAUD_38400 = 5, ///< Sets the baud rate to 38400.
BAUD_57600 = 6, ///< Sets the baud rate to 57600.
BAUD_115200 = 7 ///< Sets the baud rate to 115200.
} uart_baud;

/**
* \brief Sets the UART baud rate to one of the allowed parameters.
* Possible Baud rates:
* \li 2400
* \li 4800
* \li 9600
* \li 19200
* \li 28800
* \li 38400
* \li 57600
* \li 115200
* Returns TRUE if successful.
**/
BOOL uartX_set_baud(uart_baud rate);

/**
* \brief Returns the baud rate currently being used.
**/
uart_baud uartX_get_baud();

/**
* \brief Transmits data through UART.
* \param c The character to be sent out.
**/

void uartX_tx(char c);

/**
* \brief Returns if the next transmit call will block.
* FALSE - Non Blocking.
* TRUE - Blocking.
**/

BOOL uartX_tx_will_block();

Copy link
Owner

Choose a reason for hiding this comment

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

You have used uartX_check_receive_buffer for receiving but uartX_check_blocking for transmitting...

Copy link
Author

Choose a reason for hiding this comment

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

Yes. The receive is always non blocking. Atleast I think this is the way it would be since rx is only via timers, and this is by default non blocking

Copy link
Owner

Choose a reason for hiding this comment

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

If you call uart_receive() and there is no byte waiting, you should block until a byte is received.

Copy link
Author

Choose a reason for hiding this comment

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

Oh, I didnt know that. That would mean a while() wait inside the implementation. What would be the advantage of this? I get blocking UART is really quick. But why a blocking receive?

Copy link
Owner

Choose a reason for hiding this comment

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

Because you want the API to be consistent. If sending is blocking, then receiving should be blocking.

You can do non-blocking receiving/sending with the check functions.

/**
* \brief Returns how many more bytes can be loaded into the transmit buffer.
**/
BYTE uartX_tx_queue_len();

/**
* \brief Receives data through UART.
* Returns one byte at a time from the receive queue.
**/
char uartX_rx();

/**
* \brief Returns if the next receive call will block.
* FALSE - Non Blocking.
* TRUE - Blocking.
**/
BOOL uartX_rx_will_block();

/**
* \brief Returns count number of bytes present in the receive buffer.
**/
BYTE uartX_rx_queue_len();

Copy link
Owner

@mithro mithro Jun 24, 2016

Choose a reason for hiding this comment

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

What do you think of having a "U_ANY" and/or "U_FASTEST" maybe?

Maybe these should probably be prefixed with BAUD_ rather than just U_?

#endif