Created a UART API definition #17

RacingTornado · 2016-06-24T03:32:36Z

This defines a robust and flexible UART API which allows the example code to access the UART functionality in a standard manner irrespective of how the device is actually implementing this functionality.

mithro · 2016-06-24T04:06:40Z

include/softuart.h

@@ -0,0 +1,94 @@
+/** \file softuart.h


I recommend we go with include/uart/api.h

mithro · 2016-06-24T04:13:55Z

include/softuart.h

+ * This function uart0_receive is basically empty for fast_uart. However, in case of  
+ * timer based UART. It reads data from the queue, and returns a single character which 
+ * can then be used by the calling program.
+ *


See my comment about blocking verse non-blocking above. I think we probably need some type of function to check if there is a byte to receive.

mithro · 2016-06-24T05:14:21Z

include/uart/api.h

+ * \brief initalizes UART.
+ * This function uartX_init accepts the baud_rate and the mask.Returns 0 if successful.
+ * The mask indicates where the UART_TX pin must be attached. The rx_mask performs a 
+ * similar function. If FAST_UART is used, only transmit is allowed.


You are still talking about implementations here.

mithro · 2016-06-24T05:35:45Z

include/uart/api.h

+
+/**
+ * \brief initalizes UART.
+ * This function accepts mask for rx and tx.Returns 0 if successful.


Always put a space after a full stop.

mithro · 2016-06-24T05:41:16Z

Getting pretty close, I'm wondering if we should have shorter names than transmit and receive. Maybe rx and tx?

mithro · 2016-06-25T05:51:13Z

include/uart/api.h

+
+/**
+ * \brief Returns how many more bytes can be loaded
+ * into the buffer


Why does this comment wrap? You should be wrapping at 80 characters (or maybe 75).

mithro · 2016-06-25T05:52:03Z

You are still not putting full stops at the end of all your sentences in the documentation.

RacingTornado · 2016-06-25T06:20:36Z

You are still not putting full stops at the end of all your sentences in the documentation.

Fixed

mithro · 2016-06-27T04:57:55Z

include/uart/api.h

+ **/
+BOOL uartX_init(enum uart_baud rate, ...);
+
+/**


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

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?

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.

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.

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

mithro · 2016-06-27T04:59:30Z

Please fix the pull request description.

mithro · 2016-07-03T07:12:18Z

If you fix the enum documentation, this can be merged.

mithro · 2016-07-04T08:49:36Z

include/uart/api.h

+ * Enum values greater than 0 are supported.
+**/
+enum uart_baud {
+    BAUD_FASTEST = -2,	///< The fastest BAUD available on the system(Currently on 48Mhz,it is 1.71Mbps)


See commenting is important -- we had different understanding of this value. I think it should be "The fastest baud rate a UART supports".

Again, you shouldn't be referencing real implementations in the API document.

mithro · 2016-07-05T12:00:42Z

examples/lights/lights.c

@@ -20,6 +20,7 @@

 #include <lights.h>
 #include <delay.h>
+#include <uart/api.h>


mithro · 2016-07-11T04:41:21Z

include/uart/api.h

+ * \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


Space after full stop.

…access UART irrespective of the manner in which it is implemented. Updated

RacingTornado · 2016-07-20T06:20:17Z

I have changed things to the best of my knowledge.

RacingTornado force-pushed the merge_uart_api branch from 916a82a to 491be57 Compare June 24, 2016 03:54

mithro reviewed Jun 24, 2016
View reviewed changes

RacingTornado force-pushed the merge_uart_api branch from 491be57 to e6d4371 Compare June 24, 2016 04:13

mithro reviewed Jun 24, 2016
View reviewed changes

RacingTornado force-pushed the merge_uart_api branch 2 times, most recently from 572d7cd to 9685276 Compare June 24, 2016 04:46

mithro reviewed Jun 24, 2016
View reviewed changes

RacingTornado force-pushed the merge_uart_api branch 3 times, most recently from 3083e9c to 075d0ab Compare June 24, 2016 05:29

mithro reviewed Jun 24, 2016
View reviewed changes

mithro reviewed Jun 25, 2016
View reviewed changes

RacingTornado force-pushed the merge_uart_api branch from 7301453 to dbba734 Compare June 25, 2016 06:20

mithro changed the title ~~Created a UART API definition for allowing proper abstraction~~ Created a UART API definition Jun 27, 2016

mithro reviewed Jun 27, 2016
View reviewed changes

RacingTornado force-pushed the merge_uart_api branch from dbba734 to 3342242 Compare June 27, 2016 05:10

RacingTornado force-pushed the merge_uart_api branch from 3342242 to 4be57ba Compare July 4, 2016 07:45

mithro reviewed Jul 4, 2016
View reviewed changes

RacingTornado force-pushed the merge_uart_api branch from 4be57ba to e5de956 Compare July 5, 2016 05:57

mithro reviewed Jul 5, 2016
View reviewed changes

examples/lights/lights.c

@@ -20,6 +20,7 @@

#include <lights.h>

#include <delay.h>

#include <uart/api.h>

Copy link

Owner

mithro Jul 5, 2016

Choose a reason for hiding this comment

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

Um?

RacingTornado force-pushed the merge_uart_api branch 2 times, most recently from 84e7117 to dadddb1 Compare July 5, 2016 13:19

mithro reviewed Jul 11, 2016
View reviewed changes

RacingTornado force-pushed the merge_uart_api branch 3 times, most recently from d4322d3 to b95e144 Compare July 20, 2016 06:13

Created a UART API definition in softuart.h to allow the firmware to …

a3c3e01

…access UART irrespective of the manner in which it is implemented. Updated

RacingTornado force-pushed the merge_uart_api branch from b95e144 to a3c3e01 Compare July 20, 2016 06:16

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Created a UART API definition #17

Created a UART API definition #17

RacingTornado commented Jun 24, 2016 •

edited

Loading

mithro Jun 24, 2016

RacingTornado Jun 24, 2016

mithro Jun 24, 2016

RacingTornado Jun 24, 2016

mithro Jun 24, 2016

RacingTornado Jun 24, 2016

mithro Jun 24, 2016

RacingTornado Jun 24, 2016

mithro commented Jun 24, 2016

mithro Jun 25, 2016 •

edited

Loading

mithro commented Jun 25, 2016

RacingTornado commented Jun 25, 2016

mithro Jun 27, 2016

RacingTornado Jun 27, 2016

mithro Jul 3, 2016

RacingTornado Jul 4, 2016

RacingTornado Jul 4, 2016

mithro Jul 4, 2016

RacingTornado Jul 5, 2016

mithro commented Jun 27, 2016

mithro commented Jul 3, 2016

mithro Jul 4, 2016

mithro Jul 4, 2016

RacingTornado Jul 5, 2016

mithro Jul 5, 2016

mithro Jul 11, 2016

RacingTornado commented Jul 20, 2016

Created a UART API definition #17

Are you sure you want to change the base?

Created a UART API definition #17

Conversation

RacingTornado commented Jun 24, 2016 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mithro commented Jun 24, 2016

mithro Jun 25, 2016 • edited Loading

Choose a reason for hiding this comment

mithro commented Jun 25, 2016

RacingTornado commented Jun 25, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mithro commented Jun 27, 2016

mithro commented Jul 3, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

RacingTornado commented Jul 20, 2016

RacingTornado commented Jun 24, 2016 •

edited

Loading

mithro Jun 25, 2016 •

edited

Loading