Skip to content
This repository was archived by the owner on Sep 6, 2023. It is now read-only.

Conversation

@MrSurly
Copy link
Contributor

@MrSurly MrSurly commented Apr 4, 2017

With changes as discussed in other PR.

Also, the baudrate worked fine for me, using SPI1 and all native pins.

Copy link
Member

@dpgeorge dpgeorge left a comment

Choose a reason for hiding this comment

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

Apart from the single comment it looks good!

Also, please confirm that all code was your own, or derived from this code base.

// if a port didn't define MSB/LSB constants then provide them
#ifndef MICROPY_PY_MACHINE_SPI_MSB
#define MICROPY_PY_MACHINE_SPI_MSB (0)
#define MICROPY_PY_MACHINE_SPI_LSB (1)
Copy link
Member

Choose a reason for hiding this comment

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

These should be moved to esp32/mpconfigport.h; see stmhal/mpconfigport.h for hints.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Got it. Will update tomorrow.

esp32/mpconfigport.h: Moved SPI constants to mpconfigport.h
@MrSurly
Copy link
Contributor Author

MrSurly commented Apr 5, 2017

Haven't tested the last commit, yet. My ESP32 is at work.

@MrSurly
Copy link
Contributor Author

MrSurly commented Apr 5, 2017

Also, please confirm that all code was your own, or derived from this code base.

Yes, all mine; I only looked at the ESP-IDF documentation, and ESP-IDF code examples, and the MicroPython source files. There aren't any licensing issues.

@MrSurly
Copy link
Contributor Author

MrSurly commented Apr 5, 2017

Okay, tested. Should be good to go.

@dpgeorge
Copy link
Member

dpgeorge commented Apr 6, 2017

Thank you! Rebased, squashed and merged in 38c6e55.

@dpgeorge dpgeorge closed this Apr 6, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants