Add fixes and improvements from Micropython#120
Add fixes and improvements from Micropython#120andrewleech wants to merge 6 commits intoWiznet:micropythonfrom
Conversation
Originally by @dpgeorge from: micropython/micropython@7da9145
No change to any function names by default. 3rd party projects can set a compiler define of WIZCHIP_PREFIXED_EXPORTS=1 to rename all exported names from socket, eg "close" becomes "wizchip_close". Originally by @dpgeorge from: micropython/micropython@9d2bf9c
#define WIZCHIP_YIELD() can be globally set to yield from thread to improve the performance of connect, send, recv, sento and recvfrom. Originally by liweiwei@yeweitech.com from: micropython/micropython@73e387c
Enabling WIZCHIP_USE_MAX_BUFFER will make the TX/RX buffers the maximum available size, for use with MACRAW mode. Adapted from original version by @dpgeorge at: micropython/micropython@cd9de63
1a16731 to
0803fc5
Compare
|
Sorry. We can't merge micropython related code here. |
|
Hi @irinakim12 this PR does not include any micropython specific code, it was carefully cleaned up to only include C changes that should be relevant to any users of this library. I listed micropython in the title simply because these were commits / changes originally done by other's under the micropython project. |
|
Hi @irinakim12 I'm from the MicroPython project. We're currently using @andrewleech's fork of this repo but would prefer to point our submodule to the official repo instead. As Andrew said, this PR doesn't include anything MicroPython-specific, rather it's just general improvements that make it easy for a project (such as MicroPython) to embed this library. (In particular, avoiding name collisions with other functions). |
|
@andrewleech Could I ask you for an answer ? :) |
|
Sure, what's the question? |
I write the review about your Pull request last week. |
Sorry I can't see any review comments on this PR? |
| //***************************************************************************** | ||
| #include <string.h> | ||
| #include "socket.h" | ||
| #include "wizchip_conf.h" |
There was a problem hiding this comment.
- "#inlclude wizchip_conf.h" is already assigned in the socket.h
- we didn't use the memset.
Please delete this phrase.
| // socket, and trying to get POSIX behaviour we return 0 because: | ||
| // "If no messages are available to be received and the peer has per? | ||
| // formed an orderly shutdown, recv() shall return 0". | ||
| // TODO this return value clashes with SOCK_BUSY in non-blocking mode. |
There was a problem hiding this comment.
Please delete the comments
| void (*_write_byte) (uint8_t wb); | ||
| void (*_read_burst) (uint8_t* pBuf, uint16_t len); | ||
| void (*_write_burst) (uint8_t* pBuf, uint16_t len); | ||
| void (*_write_burst) (const uint8_t* pBuf, uint16_t len); |
There was a problem hiding this comment.
Why use const values only when using write_bust function?
Any special reason?
| if(len == 0) return SOCKERR_DATALEN; \ | ||
| }while(0); \ | ||
|
|
||
| void WIZCHIP_EXPORT(socket_reset)(void) { |
There was a problem hiding this comment.
Why do you need to use a reset function?
| #endif | ||
| } | ||
|
|
||
| WIZCHIP_EXPORT(socket_reset)(); |
There was a problem hiding this comment.
Why is the reset function only used here?
sorry. Review of this PR has been put on hold. Now you can check out my comments. |
An earlier version of this Wiznet driver has been in use in micropython (primarily on the stm32 port) since 2014, with a few additions being made to it since.
As part of my work in micropython/micropython#8540 I'm updating the driver to use the current version from here, preferably as a submodule to make future updates easier to maintain. For this to happen smoothly though, I'm hoping to submit a clean version of the micropython patches here.
There are a couple of commits here that as-is will present no change, but instead allow third part projects like micropython to integrate the library more smoothly.
A couple of other commits add features / bug fixes found previously. Hopefully the commit descriptions provide enough context for these changes.