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.

Sign up for GitHub

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

Jump to bottom

Add ESP8266 driver v1.7 #8689

Merged
merged 2 commits into from
Nov 22, 2018
Merged

Add ESP8266 driver v1.7 #8689

merged 2 commits into from
Nov 22, 2018

Conversation

VeijoPesonen
Copy link
Contributor

@VeijoPesonen VeijoPesonen commented Nov 9, 2018 •
edited
Loading

Description

Most notable changes

  • Support for the latest firmware - ESP8266 AT Bin V1.7.0.
  • Possibility to enable hardware flow control.
  • Pin config is mandatory if using a board which doesn't conform to Arduino form factor
  • Multiple bugfixes...

This is a breaking change - users are expected to check the README.md file what needs to be done.

Configuration options - mandatory ones.

Users should be aware of the fact that new implementation restricts how much heap can be used for storing data packets - socket-bufsize parameter was introduced and it's set to 8K by default. Once the limit is reached packets are dropped - doesn't matter is it a TCP or UDP socket. For TCP sockets this is fatal but an application using UDP socket should be able to handle it. This is a breaking change from user point of view because earlier the usage of heap was unrestricted.

{
    "name": "esp8266",
    "config": {
        "tx": {
            "help": "TX pin for serial connection",
            "value": null <- 'D1' by default if Arduino, adjust based on your board
        },
        "rx": {
            "help": "RX pin for serial connection",
            "value": null <- 'D0' by default if Arduino, adjust based on your board
        },
        "provide-default": {
            "help": "Provide default WifiInterface. [true/false]",
            "value": false <- Set to 'true' if this is the interface you are using
        },
        "socket-bufsize": {
            "help": "Max socket data heap usage",
            "value": 8192 <- Without HW flow control more is better. Once the limit is reached packets are
                             dropped - does not matter is it TCP or UDP.
        }
    }
}

Detailed changes

Pull request type

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

@VeijoPesonen
Copy link
Contributor Author

Test results

+--------------+---------------+-------------------------+---------------------------------------+--------+--------+--------+--------------------+
| target       | platform_name | test suite              | test case                             | passed | failed | result | elapsed_time (sec) |
+--------------+---------------+-------------------------+---------------------------------------+--------+--------+--------+--------------------+
| K64F-GCC_ARM | K64F          | tests-netsocket-dns     | ASYNCHRONOUS_DNS                      | 1      | 0      | OK     | 0.26               |
| K64F-GCC_ARM | K64F          | tests-netsocket-dns     | ASYNCHRONOUS_DNS_CACHE                | 1      | 0      | OK     | 0.6                |
| K64F-GCC_ARM | K64F          | tests-netsocket-dns     | ASYNCHRONOUS_DNS_CANCEL               | 1      | 0      | OK     | 5.41               |
| K64F-GCC_ARM | K64F          | tests-netsocket-dns     | ASYNCHRONOUS_DNS_EXTERNAL_EVENT_QUEUE | 1      | 0      | OK     | 3.36               |
| K64F-GCC_ARM | K64F          | tests-netsocket-dns     | ASYNCHRONOUS_DNS_INVALID_HOST         | 1      | 0      | OK     | 1.2                |
| K64F-GCC_ARM | K64F          | tests-netsocket-dns     | ASYNCHRONOUS_DNS_NON_ASYNC_AND_ASYNC  | 1      | 0      | OK     | 0.76               |
| K64F-GCC_ARM | K64F          | tests-netsocket-dns     | ASYNCHRONOUS_DNS_SIMULTANEOUS         | 1      | 0      | OK     | 0.78               |
| K64F-GCC_ARM | K64F          | tests-netsocket-dns     | ASYNCHRONOUS_DNS_SIMULTANEOUS_CACHE   | 1      | 0      | OK     | 0.95               |
| K64F-GCC_ARM | K64F          | tests-netsocket-dns     | ASYNCHRONOUS_DNS_SIMULTANEOUS_REPEAT  | 1      | 0      | OK     | 49.33              |
| K64F-GCC_ARM | K64F          | tests-netsocket-dns     | ASYNCHRONOUS_DNS_TIMEOUTS             | 1      | 0      | OK     | 2.75               |
| K64F-GCC_ARM | K64F          | tests-netsocket-dns     | SYNCHRONOUS_DNS                       | 1      | 0      | OK     | 0.09               |
| K64F-GCC_ARM | K64F          | tests-netsocket-dns     | SYNCHRONOUS_DNS_INVALID               | 1      | 0      | OK     | 1.34               |
| K64F-GCC_ARM | K64F          | tests-netsocket-dns     | SYNCHRONOUS_DNS_MULTIPLE              | 1      | 0      | OK     | 0.59               |
| K64F-GCC_ARM | K64F          | tests-netsocket-tcp     | TCPSOCKET_CONNECT_INVALID             | 1      | 0      | OK     | 0.25               |
| K64F-GCC_ARM | K64F          | tests-netsocket-tcp     | TCPSOCKET_ECHOTEST                    | 1      | 0      | OK     | 4.37               |
| K64F-GCC_ARM | K64F          | tests-netsocket-tcp     | TCPSOCKET_ECHOTEST_BURST              | 1      | 0      | OK     | 32.05              |
| K64F-GCC_ARM | K64F          | tests-netsocket-tcp     | TCPSOCKET_ECHOTEST_BURST_NONBLOCK     | 1      | 0      | OK     | 32.48              |
| K64F-GCC_ARM | K64F          | tests-netsocket-tcp     | TCPSOCKET_ECHOTEST_NONBLOCK           | 1      | 0      | OK     | 5.47               |
| K64F-GCC_ARM | K64F          | tests-netsocket-tcp     | TCPSOCKET_ENDPOINT_CLOSE              | 1      | 0      | OK     | 0.97               |
| K64F-GCC_ARM | K64F          | tests-netsocket-tcp     | TCPSOCKET_OPEN_CLOSE_REPEAT           | 1      | 0      | OK     | 0.07               |
| K64F-GCC_ARM | K64F          | tests-netsocket-tcp     | TCPSOCKET_OPEN_LIMIT                  | 1      | 0      | OK     | 0.22               |
| K64F-GCC_ARM | K64F          | tests-netsocket-tcp     | TCPSOCKET_RECV_100K                   | 1      | 0      | OK     | 11.85              |
| K64F-GCC_ARM | K64F          | tests-netsocket-tcp     | TCPSOCKET_RECV_100K_NONBLOCK          | 1      | 0      | OK     | 11.28              |
| K64F-GCC_ARM | K64F          | tests-netsocket-tcp     | TCPSOCKET_RECV_TIMEOUT                | 0      | 1      | FAIL   | 1.27               |
| K64F-GCC_ARM | K64F          | tests-netsocket-tcp     | TCPSOCKET_SEND_REPEAT                 | 1      | 0      | OK     | 72.98              |
| K64F-GCC_ARM | K64F          | tests-netsocket-tcp     | TCPSOCKET_SEND_TIMEOUT                | 1      | 0      | OK     | 0.91               |
| K64F-GCC_ARM | K64F          | tests-netsocket-tcp     | TCPSOCKET_THREAD_PER_SOCKET_SAFETY    | 1      | 0      | OK     | 3.28               |
| K64F-GCC_ARM | K64F          | tests-netsocket-udp     | UDPSOCKET_ECHOTEST                    | 1      | 0      | OK     | 4.53               |
| K64F-GCC_ARM | K64F          | tests-netsocket-udp     | UDPSOCKET_ECHOTEST_BURST              | 1      | 0      | OK     | 34.79              |
| K64F-GCC_ARM | K64F          | tests-netsocket-udp     | UDPSOCKET_ECHOTEST_BURST_NONBLOCK     | 1      | 0      | OK     | 34.69              |
| K64F-GCC_ARM | K64F          | tests-netsocket-udp     | UDPSOCKET_ECHOTEST_NONBLOCK           | 1      | 0      | OK     | 6.16               |
| K64F-GCC_ARM | K64F          | tests-netsocket-udp     | UDPSOCKET_OPEN_CLOSE_REPEAT           | 1      | 0      | OK     | 0.06               |
| K64F-GCC_ARM | K64F          | tests-netsocket-udp     | UDPSOCKET_OPEN_LIMIT                  | 1      | 0      | OK     | 0.21               |
| K64F-GCC_ARM | K64F          | tests-netsocket-udp     | UDPSOCKET_SENDTO_INVALID              | 0      | 1      | FAIL   | 4.11               |
| K64F-GCC_ARM | K64F          | tests-netsocket-udp     | UDPSOCKET_SENDTO_REPEAT               | 1      | 0      | OK     | 0.67               |
| K64F-GCC_ARM | K64F          | tests-netsocket-udp     | UDPSOCKET_SENDTO_TIMEOUT              | 1      | 0      | OK     | 0.12               |
| K64F-GCC_ARM | K64F          | tests-network-interface | NETWORKINTERFACE_CONN_DISC_REPEAT     | 1      | 0      | OK     | 22.26              |
| K64F-GCC_ARM | K64F          | tests-network-interface | NETWORKINTERFACE_STATUS               | 1      | 0      | OK     | 23.41              |
| K64F-GCC_ARM | K64F          | tests-network-interface | NETWORKINTERFACE_STATUS_GET           | 1      | 0      | OK     | 24.17              |
| K64F-GCC_ARM | K64F          | tests-network-interface | NETWORKINTERFACE_STATUS_NONBLOCK      | 1      | 0      | OK     | 19.93              |
| K64F-GCC_ARM | K64F          | tests-network-wifi      | WIFI-CONNECT                          | 1      | 0      | OK     | 14.98              |
| K64F-GCC_ARM | K64F          | tests-network-wifi      | WIFI-CONNECT-DISCONNECT-REPEAT        | 1      | 0      | OK     | 56.29              |
| K64F-GCC_ARM | K64F          | tests-network-wifi      | WIFI-CONNECT-NOCREDENTIALS            | 1      | 0      | OK     | 0.05               |
| K64F-GCC_ARM | K64F          | tests-network-wifi      | WIFI-CONNECT-PARAMS-CHANNEL           | 1      | 0      | OK     | 0.1                |
| K64F-GCC_ARM | K64F          | tests-network-wifi      | WIFI-CONNECT-PARAMS-CHANNEL-FAIL      | 1      | 0      | OK     | 0.11               |
| K64F-GCC_ARM | K64F          | tests-network-wifi      | WIFI-CONNECT-PARAMS-NULL              | 1      | 0      | OK     | 0.06               |
| K64F-GCC_ARM | K64F          | tests-network-wifi      | WIFI-CONNECT-PARAMS-VALID-SECURE      | 1      | 0      | OK     | 4.78               |
| K64F-GCC_ARM | K64F          | tests-network-wifi      | WIFI-CONNECT-PARAMS-VALID-UNSECURE    | 1      | 0      | OK     | 15.95              |
| K64F-GCC_ARM | K64F          | tests-network-wifi      | WIFI-CONNECT-SECURE                   | 1      | 0      | OK     | 4.79               |
| K64F-GCC_ARM | K64F          | tests-network-wifi      | WIFI-CONNECT-SECURE-FAIL              | 1      | 0      | OK     | 14.85              |
| K64F-GCC_ARM | K64F          | tests-network-wifi      | WIFI-CONSTRUCTOR                      | 1      | 0      | OK     | 0.05               |
| K64F-GCC_ARM | K64F          | tests-network-wifi      | WIFI-GET-RSSI                         | 0      | 1      | FAIL   | 13.21              |
| K64F-GCC_ARM | K64F          | tests-network-wifi      | WIFI-SCAN                             | 1      | 0      | OK     | 17.23              |
| K64F-GCC_ARM | K64F          | tests-network-wifi      | WIFI-SCAN-NULL                        | 1      | 0      | OK     | 17.31              |
| K64F-GCC_ARM | K64F          | tests-network-wifi      | WIFI-SET-CHANNEL                      | 1      | 0      | OK     | 0.09               |
| K64F-GCC_ARM | K64F          | tests-network-wifi      | WIFI-SET-CREDENTIAL                   | 1      | 0      | OK     | 0.06               |
+--------------+---------------+-------------------------+---------------------------------------+--------+--------+--------+--------------------+

@VeijoPesonen
Copy link
Contributor Author

@ARMmbed/mbed-os-ipcore please review.

@VeijoPesonen
Copy link
Contributor Author

@marcuschangarm, @JanneKiiskila, @teetak01, @tommikas, @dlfryar, @karsev for your information.

@0xc0170 0xc0170 requested a review from a team November 9, 2018 12:53
@0xc0170
Copy link
Contributor

0xc0170 commented Nov 9, 2018

@VeijoPesonen This breaks travis littlefs tests, please review

@VeijoPesonen
Copy link
Contributor Author

@0xc0170 Fix provided

@VeijoPesonen
Copy link
Contributor Author

@0xc0170 would you please trigger the build.

@adbridge adbridge requested a review from a team November 14, 2018 15:07
@adbridge
Copy link
Contributor

adbridge commented Nov 14, 2018 •
edited
Loading

@VeijoPesonen PR request type should have one and only one box selected ! I have fixed this for you on this occasion.

adbridge
adbridge approved these changes Nov 14, 2018
View reviewed changes
Copy link
Contributor

@adbridge adbridge left a comment

Choose a reason for hiding this comment

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

Looks ok to me

@adbridge
Copy link
Contributor

@AnotherButler would you like to review the documentation updates ?

@VeijoPesonen
Copy link
Contributor Author

@adbridge sorry for the request type mistake. @AnotherButler, @melwee01 has checked already the README.md so it should be fine.

@0xc0170 0xc0170 requested a review from bulislaw November 15, 2018 15:13
0xc0170
0xc0170 reviewed Nov 15, 2018
View reviewed changes
Copy link
Contributor

@0xc0170 0xc0170 left a comment •
edited
Loading

Choose a reason for hiding this comment

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

To get better understanding - can you describe the breaking changes here? I read the readme (mentions firmware update) but also noticed esp object changes (contructor for instance). What is the drive to have these changes - brief description helping reviewers to understand the drive

@cmonr
Copy link
Contributor

cmonr commented Nov 15, 2018

@0xc0170 Might want to ask @adbridge since she put the label.

@cmonr
Copy link
Contributor

cmonr commented Nov 15, 2018

Oh, nvm. @VeijoPesonen also mentioned that this was a breaking change.

@VeijoPesonen
Copy link
Contributor Author

@cmonr, @0xc0170 PR description updated to explain why this is a breaking change from user point of view. It's not a an ABI break so maybe I shouldn't have requested the breaking change label in the first place. Summa summarum, this is a breaking change from user point of view as it requires extra work from him. But this is not a breaking change from OS point of view as it doesn't change interfaces or require changes to any other components.

@cmonr
Copy link
Contributor

cmonr commented Nov 16, 2018

/morph build

@mbed-ci
Copy link

mbed-ci commented Nov 16, 2018

Build : SUCCESS

Build number : 3649
Build artifacts/logs : http://mbed-os.s3-website-eu-west-1.amazonaws.com/?prefix=builds/8689/

Triggering tests

/morph test
/morph export-build
/morph mbed2-build

@mbed-ci
Copy link

mbed-ci commented Nov 17, 2018

Exporter Build : SUCCESS

Build number : 3256
Build artifacts/logs : http://mbed-os.s3-website-eu-west-1.amazonaws.com/?prefix=builds/exporter/8689/

@mbed-ci
Copy link

mbed-ci commented Nov 17, 2018

Test : FAILURE

Build number : 3429
Test logs :http://mbed-os-logs.s3-website-us-west-1.amazonaws.com/?prefix=logs/8689/3429

@0xc0170 0xc0170 mentioned this pull request Nov 19, 2018
Closed
@0xc0170
Copy link
Contributor

0xc0170 commented Nov 19, 2018

Info: This PR has been re-bundled into a new rollup PR (#8800).

No further work is needed here, as once that PR is merged, this PR will also be closed and marked as merged.
If any more commits are made in this PR, this PR will remain open and have to go through CI on its own.

@cmonr
Copy link
Contributor

cmonr commented Nov 22, 2018

Starting CI.

@mbed-ci
Copy link

mbed-ci commented Nov 22, 2018

Test run: SUCCESS

Summary: 4 of 4 test jobs passed
Build number : 4
Build artifacts
Build logs

@0xc0170
Copy link
Contributor

0xc0170 commented Nov 22, 2018 •
edited
Loading

@AnotherButler @melwee01 This is now ready for integration. Can you please review and we can fix the readme in a separate PR (leave comments here, @VeijoPesonen create a new PR if anything).

@0xc0170 0xc0170 removed the rollup PR label Nov 22, 2018
@0xc0170
Copy link
Contributor

0xc0170 commented Nov 22, 2018

I've just noticed now

@adbridge sorry for the request type mistake. @AnotherButler, @melwee01 has checked already the README.md so it should be fine.

OK !

@0xc0170 0xc0170 merged commit ab1a723 into ARMmbed:master Nov 22, 2018
@cmonr cmonr removed the risk: G label Nov 26, 2018
@cmonr cmonr mentioned this pull request Dec 28, 2018
Merged
@VeijoPesonen VeijoPesonen deleted the esp8266-driver_v1.7 branch January 14, 2019 12:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@0xc0170 0xc0170 0xc0170 left review comments

@adbridge adbridge adbridge approved these changes

@SeppoTakalo SeppoTakalo SeppoTakalo approved these changes

@AnotherButler AnotherButler Awaiting requested review from AnotherButler

@bulislaw bulislaw Awaiting requested review from bulislaw

Assignees
No one assigned
Labels
BREAKING-CHANGE release-version: 5.11.0-rc1
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
7 participants
@VeijoPesonen @0xc0170 @adbridge @cmonr @mbed-ci @SeppoTakalo @NirSonnenschein