esp8266: Some API additions + misc fixes

[atx]

No description provided.

[coveralls]

Coverage remained the same at 93.83% when pulling 0a32eb1 on atalax:esp-api into cd3f252 on micropython:master.

[pfalcon]

esp8266: Enable setting CPU frequency to 160MHz

Did you test stability of this? In standard SDK, such frequency is enabled only for short periods of time for hard number crunching (SSL).

[atx]

Did you test stability of this?

I have not done any extensive testing, but the chip does not seem to be heating (more than usual) or crashing. There are no mentions of problems on the internet too, so it is probably safe.

[dpgeorge]

Thanks @atalax, lots of new features! But parts of this PR will need some thought/discussion first.

Mainly, there is existing API for things like ADC, unique id, version number, etc, and we should try to use these "standards" instead of inventing new ones.

[dpgeorge]

We don't yet have a standard for sleep modes (pyboard has pyb.stop(), cc3200 has pyb.Sleep() class, and there is a discussion to put it in the machine module), so this can stay here for now.

[atx]

Updated. I also removed ip() until #1269 gets implemented in cc3200

Edit: Ugh, apparently I suck at git and not all changes got commited, wait a few minutes...

[atx]

Ok, everything should be in order now

[coveralls]

Coverage increased (+0.0%) to 93.81% when pulling 52b3eed on atalax:esp-api into 97ce883 on micropython:master.

[coveralls]

Coverage remained the same at 93.81% when pulling 52b3eed on atalax:esp-api into 97ce883 on micropython:master.

[roger-]

Any idea when this'll get merged?

[pfalcon]

Merging big changes is always hard. Merging changes which involve splits and rename is well, pretty hard - there should be reason provided why that's needed, which is not there.

@atalax : I cherry-picked "esp8266: Add uos module", please rebase.

[pfalcon]

Merged "esp8266: Move initialization to system_init_done_cb".

@atalax : Please confirm that ADC stuff is tested.

[pfalcon]

@dpgeorge : Let's discuss https://github.com/atalax/micropython/commit/5214cb0fde2207213982e7dd066c92ab634d0ea3 "esp8266: Enable setting CPU frequency to 160MHz". So, this gets rid of a HAL function and calls vendor API directly from a micropython module. I'm +1 on that, I don't see why we should maintain any "HAL", I assume for pyboard it's just means to organize hw-related code, not that we try to maintain some kind of "HAL" API. But maybe I'm missing something.

[atx]

• there should be reason provided why that's needed, which is not there.

I was kind off afraid that eventually, it would be hard to recognize which functions do what (for example, it is not exactly obvious that esp.status() and esp.sleep_type() are wifi-related)

Please confirm that ADC stuff is tested.

Works. Note that for reading vdd33:

The 107th byte in esp_init_data_default.bin（0〜127byte）is named as
“vdd33_const“ , when TOUT pin is suspended vdd33_const must be set as
0xFF， that is 255

This is probably supposed to be done by manufacturer of the board, but I have no idea to what extent it actually is.

please rebase.

Done

[coveralls]

Coverage decreased (-0.0%) to 93.82% when pulling 3ef3cee on atalax:esp-api into 967f323 on micropython:master.

[pfalcon]

The 107th byte in esp_init_data_default.bin（0〜127byte）is named as
“vdd33_const“ , when TOUT pin is suspended vdd33_const must be set as
0xFF， that is 255

This is probably supposed to be done by manufacturer of the board, but I have no idea to what extent it actually is.

It's a bit hard to understand what the original quote is about, but to make sure we don't spur more confusion then it already has: why do you think anything there should be done by manufacturer of the board?

[atx]

It's a bit hard to understand what the original quote is about, but to make sure we don't spur more confusion then it already has: why do you think anything there should be done by manufacturer of the board?

If I understand the programming guide correctly, it seems that measuring vdd and tout voltage is mutually exclusive. As it depends on the board whether tout is used, the manufacturer should set the byte accordingly on initial firmware load.

[pfalcon]

As it depends on the board whether tout is used, the manufacturer should set the byte accordingly on initial firmware load.

Ok, to clarify, with esp8266, you're that manufacturer. What that excerpt refers to is esp_init_data_default.bin file in the SDK. Contents of that file are programmed somewhere closer to the end of the (external, SPI) flash, and any user can do it anytime (and sometimes it needs to be done, as esp8266 stores runtime params in that area too, and sometimes it can get into a weird state). As for hardware capabilities and actual semantics of this or another config byte, I can't comment, only experimentation and disassembly could show ;-).

[dpgeorge]

Let's discuss atalax@5214cb0. So, this gets rid of a HAL function and calls vendor API directly from a micropython module. I'm +1 on that

Yes, I'm fine with it also.

I don't see why we should maintain any "HAL"

For stdio stuff it's necessary so that, eg, readline and pyexec can be used across multiple ports.

I assume for pyboard it's just means to organize hw-related code, not that we try to maintain some kind of "HAL" API. But maybe I'm missing something.

Yes, for things that are not cross-port it's a way of collecting board-related code. Eg for pic16bit, mp_hal_milli_delay is just the name of the delay function.

For the specific function at hand, mp_hal_get_cpu_freq(), I did it that way in the ESP port because at that time you couldn't include the ESP headers in uPy code because of clashes with stdio (or something like that). But since this function is only ever used by modpyb.c we don't need it and can call the system provided function directly.

Summary: this patch is good to merge as-is.

[pfalcon]

"esp8266: Add a bunch of miscellaneous methods" - I'm merging this, but as preliminary implementation, subject to change (e.g. SLEEP_MODEM is a bit weird from user friendliness perspective. Modem? What modem? Well, certainly there's modulator-demodulator somewhere there. But calling setting SLEEP_WIFI or SLEEP_RF makes more sense.)

[pfalcon]

Please rebase.

[atx]

Please rebase.

Done

[chinadsf]

Build with esp8266 SDK1.1 error:

Use make V=1 or set BUILD_VERBOSE in your environment to increase build verbosity.
LINK build/firmware.elf
build/modpyb.o:(.text.pyb_freq+0x4): undefined reference to `system_get_cpu_freq'
build/modpyb.o:(.text.pyb_freq+0x8): undefined reference to `system_update_cpu_freq'
build/modpyb.o:(.text.pyb_freq+0x13): undefined reference to `system_get_cpu_freq'
build/modpyb.o: In function `pyb_freq':
modpyb.c:(.text.pyb_freq+0x52): undefined reference to `system_update_cpu_freq'
build/modesp.o:(.text.esp_sleep_type+0x0): undefined reference to `wifi_get_sleep_type'
build/modesp.o:(.text.esp_sleep_type+0xb): undefined reference to `wifi_get_sleep_type'
build/modpybadc.o:(.text.pyb_adc_read+0x0): undefined reference to `system_get_vdd33'
build/modpybadc.o: In function `pyb_adc_read':
modpybadc.c:(.text.pyb_adc_read+0x16): undefined reference to `system_get_vdd33'
build/moduos.o:(.text.os_uname+0x4): undefined reference to `system_get_sdk_version'
build/moduos.o: In function `os_uname':
moduos.c:(.text.os_uname+0x21): undefined reference to `system_get_sdk_version'
make: *** [build/firmware.elf] Error 1

[atx]

Build with esp8266 SDK1.1 error:

Strange, are you using SDK from esp-open-sdk?

[coveralls]

Coverage increased (+0.0%) to 94.14% when pulling ffc0d1c on atalax:esp-api into 7d8edef on micropython:master.

[pfalcon]

Ok, sow what's left here:

esp8266: Split modesp.c …
Also separated the wifi functions to esp.wifi submodule

2 points here:

1. Reviewing renames/splits is hard. Every contributor should keep this in mind and understand that submitting a split is a good way to have your patch stuck in review queue for month(s). Perhaps we should add this to contributor guidelines. The right way to handle renames/splits is to raise an RFC and wait for maintainers' approval. Otherwise, avoid doing any renames and splits - if your changes need them, that will be pointed out.

2. No, it's not ok to put wifi-related functionality in "esp.wifi". 2 sub-reasons:

2.1) As I elaborated previously "esp" is intended to be analog of "socket" module, but with a callback API. It's not called "espsocket" because it's also a bucket to drop other esp8266-related stuff which doesn't fit anywhere else. Drop, as a temporary bucket. No need to create structures and superstructures there. There's no "socket.wifi", so can't be "esp.wifi" either.

2.2) So, how to do it right? Here's the doc: http://docs.micropython.org/en/latest/library/network.html . All interface-level configuration in uPy happens via "network" module (@dpgeorge, @danicampora: is that still true?). So, feel free to introduce "network" module for esp8266 which follows interface as describe in the docs above. Start with just a module skeleton, move functions one by one to ease review. If not, keeping all stuff flat in "esp" is ok for time being.

Summing up: -2 from me on "esp8266: Split modesp.c"

[danicampora]

2.2) So, how to do it right? Here's the doc: http://docs.micropython.org/en/latest/library/network.html . All interface-level configuration in uPy happens via "network" module (@dpgeorge, @danicampora: is that still true?). So, feel free to introduce "network" module for esp8266 which follows interface as describe in the docs above. Start with just a module skeleton, move functions one by one to ease review. If not, keeping all stuff flat in "esp" is ok for time being.

Yes, that's still true :-)

[pfalcon]

"esp8266: Do not call espconn_create in constructor of esp.socket" is applied manually to pre-rename file, I couldn't preserve authorship due to path conflict.

[pfalcon]

Yes, that's still true :-)

Thanks.

So, all patches except one above were merged. Not closing right away to catch any feedback/objections.

[pfalcon]

Closing.