Support for USART parity. #757

TrevorN · 2015-12-07T03:18:55Z

Based off of the work of @jmekler in pull request #413, but with support for all platforms and rebased off the head of develop.

To use even/odd parity, pass the corresponding configure flag when invoking SerialN.begin

One Stop Bit:

SERIAL_8N1 -> No Parity
SERIAL_8O1 -> Odd Parity
SERIAL_8E1 -> Even Parity

Two Stop Bits:

SERIAL_8N2 -> No Parity
SERIAL_8O2 -> Odd Parity
SERIAL_8E2 -> Even Parity

bwheeler96 · 2015-12-09T07:19:59Z

Other than a few nits looks really solid. I could really use this feature support 👍

tomtruitt · 2015-12-28T15:48:41Z

Guys i really need 9n1... Is this possible?

TrevorN · 2016-01-02T23:13:30Z

@tomtruitt ~~Unfortunately it doesn't look like there is hardware support for 9N1, but~~ it should be possible to create that functionality through software. That is out of the scope of this pull request, however.

tomtruitt · 2016-01-03T06:40:43Z

@TrevorN

Hi thanks for your reply!

This is from some one at Particle support to me: "Yes. There is certainly hardware support and the framework for firmware support."

Unfortunately I don't have enough understanding to discern for myself.

Also I was provided with this link: https://github.com/spark/firmware/blob/aefb3342ed50314e502fc792f673af7a74f536f9/platform/MCU/STM32F1xx/STM32_StdPeriph_Driver/inc/stm32f10x_usart.h#L129

If this question is outside the scope of this pull request I apologize. Could you point me in a direction to proceed to learn more about if this is possible? Thank you

TrevorN · 2016-01-03T20:36:35Z

@tomtruitt Unfortunately the 9 in the 'USART_WordLength_9b' parameter includes the parity bit, so it is not possible in hardware to have a 9 bit message with a 10th parity bit. If you are looking to transmit an 8 bit message with an additional parity bit, this pull request will satisfy that requirement.

In the datasheet for this processor:

TrevorN · 2016-01-03T20:59:30Z

@m-mcgowan This pull request has been tested on the Photon and is functional, are there any changes that need to be made before this can be merged into develop? Serial parity is an essential feature, and my team–among others–requires it for our product.

Thanks for your consideration!

m-mcgowan · 2016-01-03T21:14:17Z

I would change the config param from uint8_t to uint32_t to provide more room for additional flags.

On the stm32f2xx we use dynamic linking. Please see the contributions doc which outlines the changes permitted. Speicifcally, it's not possible to add a parameter to an existing function. Rather, a new function should be added to the end of the dynamic linking table. (E.g. HAL_USART_BeginConfig). As well as taking a uint32_t configuration parameter, it's a good idea to add a void* parameter for future expansion.

m-mcgowan · 2016-01-14T01:31:34Z

hal/src/core/usart_hal.c

@@ -39,6 +39,8 @@ typedef enum USART_Num_Def {
 #define GPIO_Remap_None 0

 /* Private macro -------------------------------------------------------------*/
+// IS_USART_CONFIG_VALID(config) - returns true for 8 data bit, any flow control, any parity, any stop byte configurations
+#define IS_USART_CONFIG_VALID(CONFIG) ( (((CONFIG & 0b00001100)>>2) != 0b11) && (((CONFIG & 0b00110000)>>4)==0b11) )


It looks like this would fail when CONFIG==SERIAL_8N1 (0). Can you confirm please that you've tested this case?

I second the question. (CONFIG & 0b00110000)>>4)==0b11 part doesn't seem to check anything that is used in HAL_USART_BeginConfig() later, and fails for SERIAL_8N1.

flaz83 · 2016-01-27T10:12:31Z

Hi,
I should use serial 9N1, 9 data bits, no parity, 1 stop bit.
Is it possible?
probably something like that:

USART_InitStructure.USART_BaudRate = 9600;
USART_InitStructure.USART_WordLength = USART_WordLength_9b;
USART_InitStructure.USART_StopBits = USART_StopBits_1;
USART_InitStructure.USART_Parity = USART_Parity_No;
USART_InitStructure.USART_HardwareFlowControl = USART_HardwareFlowControl_None;
USART_InitStructure.USART_Mode = USART_Mode_Rx | USART_Mode_Tx;
USART_Init(USART3, &USART_InitStructure);

Thank you so much,
Flavio

tomtruitt · 2016-01-27T13:06:06Z

@TrevorN shows how much I know I thought the 9th bit was the stop bit.

@flaz83 this question was apparently answered for me earlier.

tomtruitt · 2016-01-27T13:12:38Z

@TrevorN wait I'm confused by your response. 9n1 is no parity bit so that's not an issue. Is the stop bit an issue?

flaz83 · 2016-01-27T13:12:41Z

@tomtruitt Sorry, I don't understand. I don't need a parity bit, but 9bit + stop bit.

tomtruitt · 2016-01-27T13:13:25Z

@flaz83 agreed yes I was confused by the answer as that's what I need as well

flaz83 · 2016-01-27T13:19:06Z

@tomtruitt I'm looking for 9bits + 9nth stop bit, no parity

TrevorN · 2016-01-27T13:46:34Z

@tomtruitt 8 bits plus a 9th stop bit is 8N1. The stop bit is not counted in the 8 bits.

~~@flaz83 unfortunately the hardware only has an 8 bit output register, so you can't have any packet with more than 8 data bits.~~

tomtruitt · 2016-01-27T13:54:34Z

On item number 1 of the image you uploaded it seems 9n1 would be capable. I'm sorry I'm not understanding the discrepancy

tomtruitt · 2016-01-27T13:57:27Z

@flaz83 yes i believe we are both looking for a device that is 9n1 capable with connectivity.

TrevorN · 2016-01-27T13:57:45Z

9N1 would be 9 data bits + no parity bits + 1 stop bit. This configuration ~~isn't~~ is supported. (what @flaz83 is asking for)

8N1 would be 8 data bits + no parity bits + 1 stop bit. This configuration is supported. (what @tomtruitt is asking for)

Edit: Looks like I was wrong. You can do 9 data bits, but only without parity.

TrevorN · 2016-01-27T15:26:13Z

@tomtruitt Sorry, looks like I was reading the manual incorrectly.

You can have 9 data bits, but only if there is no parity.

tomtruitt · 2016-01-27T15:41:16Z

Great this exactly what we need. I'm not very experienced with git. Should I ask my developer to create a new a pull request then? What is the next step?

TrevorN · 2016-01-27T17:45:51Z

@tomtruitt I modified the firmware to support 9N1. I currently don't have access to a photon, do you mind testing these changes with your hardware?

tomtruitt · 2016-01-28T18:29:57Z

Oh excellent! I actually don't have the hardware as I was waiting to learn of 9n1 support before purchasing.

flaz83 · 2016-01-29T11:08:22Z

Thank you so much!!!! I'm porting my software from arduino to photon and I try.
Could you please tell me how to use your code?

Serial2.begin(9600, SERIAL_9N1);
Serial2.write(0x112);
is it correct?

tomtruitt · 2016-01-29T12:29:02Z

@flaz83 I can't help but wonder if we are working on similar projects... My email is Tomtruitt@libertyvending.org if you care to chat.

… reception.

…L_USART_Begin. Modified config parameter sanity checker to permit valid configuration options.

m-mcgowan · 2016-03-10T00:07:21Z

hal/src/template/usart_hal.cpp

@@ -29,7 +29,7 @@
 void HAL_USART_Init(HAL_USART_Serial serial, Ring_Buffer *rx_buffer, Ring_Buffer *tx_buffer)
 {
 }
-void HAL_USART_Begin(HAL_USART_Serial serial, uint32_t baud)
+void HAL_USART_Begin(HAL_USART_Serial serial, uint32_t baud, uint8_t config)


I think this should be HAL_USART_BeginConfig(HAL_USART_Serial serial, uint32_t baud, uint8_t config, void*)

m-mcgowan · 2016-03-10T01:08:30Z

I've made some inline comments after making a pass through the code. Overall, this looks good, thanks for the contribution @TrevorN!

To be included in the next release, we need a few other supporting changes (as detailed in the CONTRIBUTING.md doc in the root of this repo):

a documentation PR to the spark/docs repo with documentation for the new feature
compile-time API assertions in the user/tests/wiring/api
additional test cases in the user/tests/wiring/serial_loopback to test the new usart word size, stop bit and and parity settings.

I hope that's clear - I'm happy to provide guidance where needed.

clay-to-n · 2016-03-10T01:28:54Z

@m-mcgowan Could I get some more guidance on your second point? I've found a test(api_wiring_usartserial) function in user/tests/wiring/api/wiring.cpp.

Is this where I should add additional API_COMPILE statements relating to USART configurations?

m-mcgowan · 2016-03-10T01:53:58Z

Yes, that's the right place to put some compile-time tests for the new usart config APIs.

m-mcgowan · 2016-03-14T08:34:42Z

We want to make a pre-release this week - if you can have the changes ready then we'll include this!

davismwfl · 2016-03-15T20:29:34Z

@m-mcgowan I have written the compile time assertions and written tests in the serial_loopback test. When I go to test on device for the serial loopback I get disconnected from the device and it reboots as soon as the call to Serial1.begin(...) executes. I do have a jumper between TX and RX. And this includes the 1 pre-existing test for Serial1 as well, not just my new ones.

I am sure I am missing something obvious. If I run the "Serial" test in the serial_loopback it works fine. I'd like to get this done so I can push the tests for review, so any pointers will be greatly appreciated.

m-mcgowan · 2016-03-16T07:48:52Z

I tried the serial_loopback on the develop branch and it's working for me. Be sure you've flashed system firmware to the device with the changes in this PR.

Please push your latest changes, rebased against develop and I'll help investigate.

…/TrevorN/firmware into TrevorN-feature/hal-usart-parity-options # Conflicts: # hal/inc/hal_dynalib_usart.h

Feature/hal usart parity options

clay-to-n · 2016-03-16T20:02:06Z

Created a documentation pull request for this feature here: particle-iot/docs#317

Added Assert and Serial Loopback tests for Serial Parity features.

clay-to-n · 2016-03-16T21:16:31Z

Okay, this should have tests and API assertions now thanks to @davismwfl.

davismwfl · 2016-03-16T21:23:20Z

@m-mcgowan and others. My original post and issue with the tests not running properly on the device were resolved by making sure I was building the firmware from the firmware/module directory and not firmware/main.

To add to some of my confusion the serial_loopback test in the branch prior to merging with develop would flash to the device and partially run but fail when using Serial1. After the merge with DEVELOP it would fail in SOS mode which was more telling. After re-reading the documentation once I built from firmware/modules for the firmware and firmware/main for tests everything ran properly and was testable.

m-mcgowan · 2016-03-17T20:21:02Z

Nice work everyone!

m-mcgowan · 2016-03-17T20:26:44Z

@avtolstoy Could you please review, run the tests, and merge this ASAP! Thanks! Nevermind, I did it. ;-)

m-mcgowan · 2016-03-17T20:49:24Z

Tested on Core, Photon, Electron, all green.

Support for USART parity.

clay-to-n · 2016-03-17T21:02:14Z

Woohoo!

m-mcgowan added this to the 0.4.9 milestone Jan 3, 2016

m-mcgowan reviewed Jan 14, 2016
View reviewed changes

m-mcgowan removed this from the 0.4.9 milestone Jan 19, 2016

Trevor Nielsen and others added 4 commits March 8, 2016 17:43

Added support for even and odd parity bits for usart transmission and…

35a0d03

… reception.

shorted long lines/beautified comments

541e925

HAL_USART_beginConfig no longer treads on the toes of the original HA…

2de8605

…L_USART_Begin. Modified config parameter sanity checker to permit valid configuration options.

added void pointer to HAL_USART_BeginConfig facilitate future expansion.

209d909

m-mcgowan reviewed Mar 10, 2016
View reviewed changes

organizes constants better and cleans up formatting

faff9e7

m-mcgowan and others added 2 commits March 16, 2016 09:07

Merge branch 'feature/hal-usart-parity-options' of https://github.com…

5a0f7bd

…/TrevorN/firmware into TrevorN-feature/hal-usart-parity-options # Conflicts: # hal/inc/hal_dynalib_usart.h

Merge pull request #1 from spark/feature/hal-usart-parity-options

15d4430

Feature/hal usart parity options

clay-to-n mentioned this pull request Mar 16, 2016

Adds docs for serial.begin() usart config settings particle-iot/docs#317

Closed

davismwfl and others added 2 commits March 16, 2016 17:05

Added Assert and Serial Loopback tests for Serial Parity features.

33ce8d9

Merge pull request #2 from davismwfl/feature/hal-usart-parity-options

a56ecca

Added Assert and Serial Loopback tests for Serial Parity features.

m-mcgowan added this to the 0.5.x milestone Mar 17, 2016

m-mcgowan assigned avtolstoy Mar 17, 2016

m-mcgowan unassigned avtolstoy Mar 17, 2016

m-mcgowan added a commit that referenced this pull request Mar 17, 2016

Merge pull request #757 from TrevorN/feature/hal-usart-parity-options

0384e46

Support for USART parity.

m-mcgowan merged commit 0384e46 into particle-iot:develop Mar 17, 2016

technobly added the enhancement label Mar 25, 2016

technobly mentioned this pull request Apr 7, 2016

First attempt to implement user-configurable USART settings #412

Closed

m-mcgowan mentioned this pull request Apr 20, 2016

Adding support for custom serial bit parity #413

Closed

Support for USART parity. #757

Support for USART parity. #757

Conversation

TrevorN commented Dec 7, 2015

bwheeler96 commented Dec 9, 2015

tomtruitt commented Dec 28, 2015

TrevorN commented Jan 2, 2016

tomtruitt commented Jan 3, 2016

TrevorN commented Jan 3, 2016

TrevorN commented Jan 3, 2016

m-mcgowan commented Jan 3, 2016

m-mcgowan Jan 14, 2016

Choose a reason for hiding this comment

sergeuz Jan 21, 2016

Choose a reason for hiding this comment

flaz83 commented Jan 27, 2016

tomtruitt commented Jan 27, 2016

tomtruitt commented Jan 27, 2016

flaz83 commented Jan 27, 2016

tomtruitt commented Jan 27, 2016

flaz83 commented Jan 27, 2016

TrevorN commented Jan 27, 2016

tomtruitt commented Jan 27, 2016

tomtruitt commented Jan 27, 2016

TrevorN commented Jan 27, 2016

TrevorN commented Jan 27, 2016

tomtruitt commented Jan 27, 2016

TrevorN commented Jan 27, 2016

tomtruitt commented Jan 28, 2016

flaz83 commented Jan 29, 2016

tomtruitt commented Jan 29, 2016

m-mcgowan Mar 10, 2016

Choose a reason for hiding this comment

m-mcgowan commented Mar 10, 2016

clay-to-n commented Mar 10, 2016

m-mcgowan commented Mar 10, 2016

m-mcgowan commented Mar 14, 2016

davismwfl commented Mar 15, 2016

m-mcgowan commented Mar 16, 2016

clay-to-n commented Mar 16, 2016

clay-to-n commented Mar 16, 2016

davismwfl commented Mar 16, 2016

m-mcgowan commented Mar 17, 2016

m-mcgowan commented Mar 17, 2016

m-mcgowan commented Mar 17, 2016

clay-to-n commented Mar 17, 2016