Skip to content
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

Reordering - HardwareSerial Constructor #6492

Merged
merged 1 commit into from
Mar 28, 2022

Conversation

SuGlider
Copy link
Collaborator

Summary

Quick necessary fix in order to allow Lib Builder compilation.

Impact

None.

Related links

None.

@SuGlider SuGlider added this to the 2.0.3 milestone Mar 28, 2022
@SuGlider SuGlider requested a review from me-no-dev March 28, 2022 13:15
@SuGlider SuGlider self-assigned this Mar 28, 2022
@github-actions
Copy link
Contributor

Unit Test Results

0 files  0 suites   0s ⏱️
0 tests 0 ✔️ 0 💤 0

Results for commit e70c863.

Jason2866 referenced this pull request Mar 28, 2022
* Adds HardwareSerial::onReceiveTimeout()

* Fixed typo

* Changes requested

* Fix eventQueueReset

* Changed _onReceiveTimeout to _rxTimeout for consistency

* Uniform uart_set_rx_timeout condition

* test _uart not NULL in eventQueueReset()

check if _uart is not NULL before using it.

* revert last commit - no need for it

reverting last change made - it is not necessary.

* adds onReceive() parameter 

In order to allow the user to choose if onReceive() call back will be called only when UART Rx timeout happens or also when UART FIFO gets 120 bytes, 
a new parameter has been added to onReceive() with the default behavior based on timeout.

    void onReceive(OnReceiveCb function, bool onlyOnTimeout = true);

   onReceive will setup a callback that will be called whenever an UART interruption occurs (UART_INTR_RXFIFO_FULL or UART_INTR_RXFIFO_TOUT)
   UART_INTR_RXFIFO_FULL interrupt triggers at UART_FULL_THRESH_DEFAULT bytes received (defined as 120 bytes by default in IDF)
   UART_INTR_RXFIFO_TOUT interrupt triggers at UART_TOUT_THRESH_DEFAULT symbols passed without any reception (defined as 10 symbos by default in IDF)
   onlyOnTimeout parameter will define how onReceive will behave:
   Default: true -- The callback will only be called when RX Timeout happens. 
                           Whole stream of bytes will be ready for being read on the callback function at once.
                           This option may lead to Rx Overflow depending on the Rx Buffer Size and number of bytes received in the streaming
            false --    The callback will be called when FIFO reaches 120 bytes and also on RX Timeout.
                           The stream of incommig bytes will be "split" into blocks of 120 bytes on each callback.
                           This option avoid any sort of Rx Overflow, but leaves the UART packet reassembling work to the Application.

* Adds onReceive() parameter for timeout only

* Adds back setRxTimeout()

* Adds setRxTimeout()

* CI Syntax error - "," missing

Co-authored-by: Rodrigo Garcia <[email protected]>
@mrengineer7777
Copy link
Collaborator

I don't see how moving _onReceiveErrorCB changes compilation, but it does look better.
On the other hand, defining "_lock" definitely helps :)

@me-no-dev me-no-dev merged commit aa783e6 into espressif:master Mar 28, 2022
Copy link
Collaborator

@Jason2866 Jason2866 left a comment

Choose a reason for hiding this comment

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

Arduino Lib Builder does compile successfull with this changes
https:/Jason2866/esp32-arduino-lib-builder/actions/runs/2052595611

@SuGlider SuGlider deleted the quick-fix-Lib_Builder branch March 28, 2022 15:02
@mrengineer7777
Copy link
Collaborator

@mrengineer7777 https:/espressif/esp32-arduino-lib-builder/runs/5719711169?check_suite_focus=true#step:4:51805

Ah, so it has to do with the order in which variables are initialized in the constructor. This is a C++ concept I don't yet understand, but I can see that declaring the variables in the same order in both the header and constructor makes the compiler happy.
Snip from err log below:

 In file included from /home/runner/work/esp32-arduino-lib-builder/esp32-arduino-lib-builder/components/arduino/cores/esp32/HardwareSerial.cpp:7:
/home/runner/work/esp32-arduino-lib-builder/esp32-arduino-lib-builder/components/arduino/cores/esp32/HardwareSerial.h: In constructor 'HardwareSerial::HardwareSerial(int)':
/home/runner/work/esp32-arduino-lib-builder/esp32-arduino-lib-builder/components/arduino/cores/esp32/HardwareSerial.h:167:22: error: 'HardwareSerial::_onReceiveErrorCB' will be initialized after [-Werror=reorder]
     OnReceiveErrorCb _onReceiveErrorCB;

Good fix.

Jason2866 added a commit to Jason2866/arduino-esp32 that referenced this pull request Mar 31, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Development

Successfully merging this pull request may close these issues.

4 participants