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

Enable json overriding ESP8266 default baud rate #11302

Merged

Conversation

@desmond-blue
Copy link
Contributor

commented Aug 23, 2019

Description

Got a question here about configuring ESP8266_DEFAULT_BAUD_RATE in mbed_app.json, I think a small modification is able to achieve that.

Pull request type

[x] Fix
[ ] Refactor
[ ] Target update
[ ] Functionality change
[ ] Docs update
[ ] Test update
[ ] Breaking change

Reviewers

Release Notes

@0xc0170

This comment has been minimized.

Copy link
Member

commented Aug 23, 2019

This should fix #11300 (same as url reported above)

@ciarmcom ciarmcom requested review from ARMmbed/mbed-os-maintainers Aug 23, 2019
@ciarmcom

This comment has been minimized.

Copy link
Member

commented Aug 23, 2019

@desmond-blue, thank you for your changes.
@ARMmbed/mbed-os-ipcore @ARMmbed/mbed-os-maintainers please review.

@michalpasztamobica

This comment has been minimized.

Copy link
Contributor

commented Aug 23, 2019

Thanks for the PR @desmond-blue !
I wonder however if it wouldn't be neater to accomplish this in a similar manner we do for other configurable items, such as pins or buffer size?
They usually get added to mbed_lib.json, which means they are available in the code as MBED_CONF_ESP8266_XXX_YYY, for example here, and can be set in mbed_app.json per target with esp8266.xxx-yyy.

@VeijoPesonen , what do you think?

@teetak01

This comment has been minimized.

Copy link
Contributor

commented Aug 23, 2019

It would make sense to add entry to mbed_lib.json and define there the default baudrate.

Copy link
Contributor

left a comment

Please use mbed_lib.json instead

@@ -33,7 +33,9 @@

#define TRACE_GROUP "ESPA" // ESP8266 AT layer

#ifndef ESP8266_DEFAULT_BAUD_RATE

This comment has been minimized.

Copy link
@AnttiKauppila

AnttiKauppila Aug 23, 2019

Contributor

Modify https://github.com/ARMmbed/mbed-os/blob/master/components/wifi/esp8266-driver/mbed_lib.json
to have one extra config like:
"baudrate": { "help": "Baudrate for ESP8266, defaults to 115200", "value": 115200 }
Then in code you can use "MBED_CONF_ESP8266_BAUDRATE" instead of defining "ESP8266_DEFAULT_BAUD_RATE"

This comment has been minimized.

Copy link
@michalpasztamobica

michalpasztamobica Aug 23, 2019

Contributor

To avoid confusion perhaps this config could be called uart_baudrate or serial_baudrate, to avoid an impression that this directly influences the WiFi communication speed, which might be a primary interest of many users.
I realize that the serial baudrate in fact throttles the module's communication speed, but I think we should be specific with the naming, whenever we can afford it :).

@star297

This comment has been minimized.

Copy link
Contributor

commented Aug 23, 2019

Could this override setting go in the same area as the ESP8266 tx and rx pin settings to keep things nice and tidy?

Perhaps an update on the ESP8266 firmware update page would be an idea, I have been using the ESP8266 at 921600 on all targets above 100Mhz for some months now, its a considerable speed improvement even without hardware control.

@desmond-blue desmond-blue force-pushed the desmond-blue:feature-config-esp8266-baud-rate branch from ab76025 to 77d403b Aug 26, 2019
@desmond-blue

This comment has been minimized.

Copy link
Contributor Author

commented Aug 26, 2019

Really appreciate the comments, I've modified it accordingly, would you review it again? Thanks.

Copy link
Contributor

left a comment

Thanks, @desmond-blue , that's exactly what I meant :)

@0xc0170 0xc0170 requested a review from AnttiKauppila Aug 26, 2019
@0xc0170 0xc0170 added needs: CI and removed needs: work labels Aug 26, 2019
@0xc0170

This comment has been minimized.

Copy link
Member

commented Aug 26, 2019

CI started

@0xc0170

This comment has been minimized.

Copy link
Member

commented Aug 26, 2019

CI started

@mbed-ci

This comment has been minimized.

Copy link

commented Aug 27, 2019

Test run: FAILED

Summary: 3 of 11 test jobs failed
Build number : 1
Build artifacts

Failed test jobs:

  • jenkins-ci/mbed-os-ci_cloud-client-test
  • jenkins-ci/mbed-os-ci_greentea-test
  • jenkins-ci/mbed-os-ci_dynamic-memory-usage
@0xc0170

This comment has been minimized.

Copy link
Member

commented Aug 28, 2019

CI restarted

@mbed-ci

This comment has been minimized.

Copy link

commented Aug 29, 2019

Test run: SUCCESS

Summary: 11 of 11 test jobs passed
Build number : 2
Build artifacts

@0xc0170 0xc0170 added ready for merge and removed needs: CI labels Aug 29, 2019
@0xc0170 0xc0170 merged commit a884c7c into ARMmbed:master Aug 29, 2019
25 checks passed
25 checks passed
continuous-integration/jenkins/pr-head This commit looks good
Details
jenkins-ci/build-ARM Success
Details
jenkins-ci/build-GCC_ARM Success
Details
jenkins-ci/build-IAR Success
Details
jenkins-ci/cloud-client-test Success
Details
jenkins-ci/dynamic-memory-usage RTOS ROM(+0 bytes) RAM(+0 bytes)
Details
jenkins-ci/exporter Success
Details
jenkins-ci/greentea-test Success
Details
jenkins-ci/mbed2-build-ARM Success
Details
jenkins-ci/mbed2-build-GCC_ARM Success
Details
jenkins-ci/mbed2-build-IAR Success
Details
jenkins-ci/unittests Success
Details
travis-ci/astyle Success!
Details
travis-ci/docs Success!
Details
travis-ci/doxy-spellcheck Success!
Details
travis-ci/events Success! Runtime is 8684 cycles.
Details
travis-ci/gitattributestest Success!
Details
travis-ci/include_check Success!
Details
travis-ci/licence_check Success!
Details
travis-ci/littlefs Success! Code size is 8464B.
Details
travis-ci/psa-autogen Success!
Details
travis-ci/tools-py2.7 Success!
Details
travis-ci/tools-py3.5 Success!
Details
travis-ci/tools-py3.6 Success!
Details
travis-ci/tools-py3.7 Success!
Details
@star297

This comment has been minimized.

Copy link
Contributor

commented Oct 7, 2019

Has this fix been merged with the latest Mbed-os (online version 5.14) yet?
I have tried testing with the HTTP-example as per my comments here:
https://os.mbed.com/questions/86806/How-to-change-ESP8266-default-WiFi-baud-/
but no luck, but I'm probably doing it wrong.

@michalpasztamobica

This comment has been minimized.

Copy link
Contributor

commented Oct 8, 2019

Hi @star297 , yes, it is available in mbed-os 5.14. Online version should be just the same.
However, in the process of code review (see above) some small modifications were made and the variable that you need to modify in your mbed_app.json file is actually called serial-baudrate and not esp8266-baudrate.

This diagnosis is just my blind guess. If I guessed wrong and you still get "no luck", please provide your mbed_app.json file and a more detailed description of the exact error you are getting (such as compilation output. I am sure we'll work it out :) .

@star297

This comment has been minimized.

Copy link
Contributor

commented Oct 8, 2019

Hi Michal, yes, I have that set to serial-baudrate but not working, if you scroll down to the bottom of my question you will see the the mbed_app.json file in detail.

https://os.mbed.com/questions/86806/How-to-change-ESP8266-default-WiFi-baud-/

It may simply be I have that in the wrong section.

Terminal output:

Mbed OS version 5.13.4

[NWKH] Connecting to network...
[NWKH] Failed to connect to network (-3012)
Cannot connect to the network, see serial output

That Mbed OS version is a bit odd, I have updated and checked version is 5.14 in the Revision History. (Side tracking, Jimmy Brisson has an issue with his PC clock his entries are always 14 Feb 2017, I do find that a bit confusion when switching revisions.)

Compilation output:

Warning: L3912W: Option 'legacyalign' is deprecated.
Warning: 'MBED_RAM_START' macro redefined [-Wmacro-redefined] in "tmp/A4VEuA", Line: 44, Col: 9
Warning: 'MBED_RAM_SIZE' macro redefined [-Wmacro-redefined] in "tmp/A4VEuA", Line: 45, Col: 9
Success: Success!

@michalpasztamobica

This comment has been minimized.

Copy link
Contributor

commented Oct 8, 2019

Sorry, I didn't catch where the mbed_app.json was...

To use this option you need to add it like this:

"target_overrides": {
        "*": {
            "esp8266.serial-baudrate": 921600,

This will configure the baudrate for all targets. You can put it under any of your targets instead (for example under "NUCLEO_F401RE").

@star297

This comment has been minimized.

Copy link
Contributor

commented Oct 8, 2019

I get this compile error:

Error: Attempt to override undefined parameter 'esp8266.serial-baudrate' in 'application[*]'
Info: Unable to download. Fix the reported errors...

@michalpasztamobica

This comment has been minimized.

Copy link
Contributor

commented Oct 8, 2019

Are you sure you are compiling for a platform that uses esp8266?
Can you please try to put it into the section of a target that surely uses esp8266? Pereferably, right after "esp8266.provide-default" : true?

I tried it on my setup and compilation for K64F + ESP8266 worked fine...

@star297

This comment has been minimized.

Copy link
Contributor

commented Oct 8, 2019

mbed-os revision 6691 is the problem, switch to revision 6698 and it does work, I can set the baud rate.

However, unfortunately, Mbed's ESP8266 driver is not fast enough to work at 921600, it will connect but struggles handling HTTPS, but it does work fine at 460800 which is a vast improvement over 115200.

I have also tried this:
https://os.mbed.com/teams/mbed-os-examples/code/mbed-os-example-wifi/
This also works and I do get the correct connected AP RSSI level.

Thank's for your time Michal :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
9 participants
You can’t perform that action at this time.