Skip to content

Conversation

@Lennyz1988
Copy link

Hi,

I added support for ESP32. Since this is my first pull request, I am unsure if I am doing it properly.

Kind regards,

Lennyz

Copy link

@ThomasWaldmann ThomasWaldmann left a comment

Choose a reason for hiding this comment

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

Thanks for this PR.

Some nitpicky comments from me, I'll try this code soon.

Maybe you could collapse most of your changesets (esp. the ones which all have the same commit comment).

self.spi.init()
elif board == 'esp32':
self.spi = SPI(baudrate=100000, polarity=0, phase=0, sck=self.sck, mosi=self.mosi, miso=self.miso)
self.spi.init()

Choose a reason for hiding this comment

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

as you are doing precisely the same thing as 3 lines above, how about just changing line 32 to:

elif board == 'esp8266' or board == 'esp32':

| mosi | 2 | "GP16" | |
| miso | 4 | "GP15" | |
| rst | 5 | "GP22" | |
| cs | 14 | "GP14" |Labeled SDA on most RFID-RC522 boards |

Choose a reason for hiding this comment

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

i don't have a "cs" on my board.
so, if it really is labelled "sda" for me, keeping that comment seems useful.

Choose a reason for hiding this comment

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

i can confirm that cs is labelled sda for me. at least it practically works if i wire it like that.



def do_read():
def do_read(x,y):

Choose a reason for hiding this comment

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

pep8: please have a blank after the comma

also, don't call it x and y, but give good names.

raise RuntimeError("Unsupported platform")

print("")
print('READER %s' %y)

Choose a reason for hiding this comment

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

pep8: please have a blank after the percent

@ThomasWaldmann
Copy link

@wendlers I'll test the code from this PR on my ESP32 soon.

Do you think it could be merged after some clean up and this test?

Copy link

@ThomasWaldmann ThomasWaldmann left a comment

Choose a reason for hiding this comment

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

ok, i can confirm now, that these kind of changes makes it work on esp32.

I tested the read and write example code (with the pins that worked for me) and both worked.
esp32 wems oled board, rfid-rc522 board from aliexpress.

whether the simple example code needs to support multiple readers is questionable though.

| mosi | 2 | "GP16" | |
| miso | 4 | "GP15" | |
| rst | 5 | "GP22" | |
| cs | 14 | "GP14" |Labeled SDA on most RFID-RC522 boards |

Choose a reason for hiding this comment

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

i can confirm that cs is labelled sda for me. at least it practically works if i wire it like that.

| mosi | 2 | "GP16" | 23 |
| miso | 4 | "GP15" | 19 |
| rst | 5 | "GP22" | 4 |
| cs | 14 | "GP14" | 2 |

Choose a reason for hiding this comment

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

i don't have some of these pins on my wems oled esp32 devboard. 16, 5, 26, 25, 4 worked for me.

@DDDanny
Copy link

DDDanny commented Apr 21, 2018

yolo ... sry than for #9
ESP32S NODE MCU v1.1
pin cfg as described in that issue

#HSPI BUS #NodeMCU ESP32 WROOM printed on Pin@board
ESP32_HSPI_CLOCK = 14 #P14 (HSPICLK in ESP32 Docs)
ESP32_HSPI_SLAVE_SELECT = 15 #P15 (HSPICS0 in ESP32 Docs)
ESP32_HSPI_MISO = 12 #P12 (HSPIQ[uery] in ESP32 Docs)
ESP32_HSPI_MOSI = 13 #P13 (HSPID[ata] in ESP32 Docs)

ESP32_MFRC522_RST = 3 #SD3, as plain GPIO - can be any unused GPIO

@oOHenry
Copy link

oOHenry commented Jul 13, 2018

any updates on this?

@DDDanny
Copy link

DDDanny commented Jul 13, 2018

Hi Henry,
what exactly do you mean? technical the descibed solution is working; My ESP32S is live and operating with it -> No merge so far so I manually changed that line in the local copy.

Furthermore u can use the code from @Tasm-Devil 's fork (https://github.com/Tasm-Devil/micropython-mfrc522-esp32)
There the hole block with the boards is removed and a SPI object is passed to constructor. (see in above repo: examples -> read.py ) - which is of cuase a very handy and clean approach.

Greetings from Hamburg
Danny

@oOHenry
Copy link

oOHenry commented Jul 14, 2018

Hi,
I meant the status of this merge request.
Thanks for the hint with the fork.
Greetings.

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.

4 participants