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

Make gen_esp32part.py compatible with both python2.7 and python3. #577

Closed
wants to merge 1 commit into from
Closed

Make gen_esp32part.py compatible with both python2.7 and python3. #577

wants to merge 1 commit into from

Conversation

dmopalmer
Copy link
Contributor

I ran into the issue of espressif/arduino-esp32#236

This is an step forward to make the toolchain compatible with Python3. It includes both a comparison issue and unicode/str/bytes issues.

This has been tested to the extent that 'make flash' of hello_world.c runs under both python2.7 and python 3.6.1 under OSX, and I don't see any reason why it shouldn't work under linux.

The test that I have not done is: under Windows, does the binary output to stdout work properly (and did it before)?

In the binary case, I use 'wb' as the mode for files, and use stdout.buffer instead of stdout as the writeable object . So what was windows doing when it tried to write a 0x0a (which is the byte for newline)? Did it add an 0x0d byte (to make it look like '\r\n')?

@CLAassistant
Copy link

CLAassistant commented May 6, 2017

CLA assistant check
All committers have signed the CLA.

@projectgus
Copy link
Contributor

Thanks for sending this, @dmopalmer .

FWIW, this change looks good to me (I'm the original gen_esp32part.py author.) I'd been meaning to do something like this for gen_esp32part.py and the other IDF Python tools for a while.

I expect this change will mean everything works fine with Python 3 in ESP32 Arduino. esptool.py is tested working with Python 2 & 3 already.

@projectgus
Copy link
Contributor

Oops, when I replied before I thought this PR was on the arduino repo not the IDF one.

Have cherry-picked this into our review & merge queue, with a few additional tweaks.

@projectgus projectgus added the Status: Pending blocked by some other factor label May 12, 2017
igrr pushed a commit that referenced this pull request May 16, 2017
igrr pushed a commit that referenced this pull request May 16, 2017
@projectgus
Copy link
Contributor

Merged in beffcd6, thanks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants