Modification of ArduinoCore-avr to use API #329
Conversation
Move CDC and MSC classes to own files. Fix all u8/u32 entries to stantard types.
| @@ -114,7 +115,7 @@ void HardwareSerial::_tx_udr_empty_irq(void) | |||
|
|
|||
| // Public Methods ////////////////////////////////////////////////////////////// | |||
|
|
|||
| void HardwareSerial::begin(unsigned long baud, byte config) | |||
| void UartClass::begin(unsigned long baud, uint16_t config) | |||
There was a problem hiding this comment.
The config is later assigned to a *_ucsrc which is declared as a uint8_t in UART.h. Was this change intentional?
There was a problem hiding this comment.
This is due to https://github.com/arduino/ArduinoCore-API/blob/master/api/HardwareSerial.h#L91 , so the signature must be uint16_t .
Said that, I'll push a commit to fix this by casting the assignment
*_ucsrc = (uint8_t)config; (since we are assured that config fits an uint8_t)
|
|
||
| #include "Stream.h" | ||
| #if ARDUINO_API_VERSION > 10000 | ||
| using namespace arduino; |
There was a problem hiding this comment.
Maybe add a comment to explain why this using namespace statement is needed. It us contrary to the usual recommendation of avoiding using namespace in a C++ header file: https://stackoverflow.com/questions/5849457/using-namespace-in-c-headers
| volatile uint8_t *ubrrh, volatile uint8_t *ubrrl, | ||
| volatile uint8_t *ucsra, volatile uint8_t *ucsrb, | ||
| volatile uint8_t *ucsrc, volatile uint8_t *udr); | ||
| void begin(unsigned long baud) { begin(baud, SERIAL_8N1); } | ||
| void begin(unsigned long, uint8_t); | ||
| void begin(unsigned long, uint16_t); |
There was a problem hiding this comment.
Similar to above, it's not clear why this was changed to a uint16_t, when the config is later assigned only to a uint8_t.
|
Has there been any further discussion on this? Or generally bringing the AVR core into the API? forum or mailing list perhaps? Thanks |
The following proposed modifications are intended to port the API to the AVR boards:
Files modified:
Files deleted, because included in the API: