Skip to content

Due: Config options for USART (non-8-bit modes) now fails if used on UART #2539

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

Merged
merged 3 commits into from
Jan 19, 2015

Conversation

cmaglie
Copy link
Member

@cmaglie cmaglie commented Jan 6, 2015

This is a possible solution for #2006

On Arduino Due, Serial is an UART and doesn't support non-8bit modes for example:

Serial.begin(9600, SERIAL_E71);

Without this pull request the code above compiles without errors but it doesn't run correctly.

With this pull request the runtime error is detected at compile time (i.e. the code above doesn't compile any more) making it much easier to debug.

/cc @collin80 @bluesign2k

@bluesign2k
Copy link
Contributor

This looks like a pretty good solution. It was the sort of thing I was trying to figure out how to do, but I never got around to looking into it.

I believe there are however still some things that need looking at before this gets merged. Firstly, in the UART modes, you've given options for using 2 stop bits, but I believe the UART module doesn't support this. Additionally, both the UART and USART mode lists are missing the options for mark and space as parity.

On line 60 of UARTClass.cpp the config should only be ORed with UART_MR_CHMODE_NORMAL rather than (US_MR_USART_MODE_NORMAL | US_MR_USCLKS_MCK | US_MR_CHMODE_NORMAL) as these bits are for the USART mode register. In this case it'll actually work anyway because the bits that are implemented are in the same positions as in each of the mode registers, but I feel it would be bad practice to leave it as is. Similarly, this is the case for the modes listed in the UARTModes enum. I can see why it's been done this way, but perhaps it's also best to AND the config with 0xE00 as I did in #2006 to ensure un-implemented bits are being attempted to be written to. Again, I know it shouldn't be an issues but I have had odd problems in the past when accidentally writing to un-implemented bits in settings registers.

I'd make the changes myself but I don't know the best way to do this using github - would I fork your repo edit it then submit a pull to you rather than here?

@cmaglie
Copy link
Member Author

cmaglie commented Jan 9, 2015

@bluesign2k,
Try the following

  1. pick my commits
  2. push them in your fork
  3. push more commits with patch in your fork

when you're done
4) I'll pull from your fork and push into this branch

At this point the pull request should update itself.

C

@bluesign2k
Copy link
Contributor

Okay, so you should be able to find the updated version here: bluesign2k@7e9cf6d

@cmaglie
Copy link
Member Author

cmaglie commented Jan 13, 2015

@bluesign2k
I've commented on your commit and updated the pull request with a possible solution.

@bluesign2k
Copy link
Contributor

Woops! It's been quite a while since I looked at this - I hadn't realised that the two classes shared quite so much these days. Your modification looks fine to me although unless my eyes deceive me, lines 47 to 52 of USARTClass.cpp seem to be a repeat of lines 40 to 45 right above. Apart from that, I think this is ready to merge.

I suggest once this is merged that #2006 can also be closed too as the only extra thing it had was the defines in preparation for 9-bit modes for the USART... but I don't see much interest in that any more.

@cmaglie cmaglie merged commit 00f23d3 into arduino:ide-1.5.x Jan 19, 2015
@cmaglie cmaglie deleted the sam-usart-mode-fix branch January 19, 2015 17:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants