-
-
Notifications
You must be signed in to change notification settings - Fork 7k
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
Conversation
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? |
@bluesign2k,
when you're done At this point the pull request should update itself. C |
Okay, so you should be able to find the updated version here: bluesign2k@7e9cf6d |
@bluesign2k |
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. |
This is a possible solution for #2006
On Arduino Due, Serial is an UART and doesn't support non-8bit modes for example:
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