ports/rp2: Change MICROPY_PY_LWIP setting value and netinfo structure. #9862
Conversation
- When MICROPY_PY_LWIP is set as '1' , it can't read the register value of CREG in wiznet5k_regs function.
- Change the location of netinfo structure in the wiznet5k_init function
- Delete the define of MICROPY_PY_LWIP in the mpconfigboard.cmake of W5100S_EVB_PICO and W5500_EVB_PICO
- When cmake compile, it can't change the value of MICROPY_PY_LWIP
Delete the define of MICROPY_PY_LWIP in the mpconfigboard.cmake of W5100S_EVB_PICO and W5500_EVB_PICO. When cmake compile, it can't change the value of MICROPY_PY_LWIP extmod/network_wiznet5k.c : Change code for WIZNET5K_regs() Modified the register address area each chip in wiznet5k_regs function When MICROPY_PY_LWIP is set as '0' , it can't read the register value of CREG in wiznet5k_regs function. Change the location of netinfo structure in the wiznet5k_init function
extmod/network_wiznet5k.c : Change code for WIZNET5K_regs(). extmod/network_wiznet5k.c:Change the netinfo structure in wiznet5k_init().
Delete the define of MICROPY_PY_LWIP in the mpconfigboard.cmake of W5100S_EVB_PICO and W5500_EVB_PICO. When cmake compile, it can't change the value of MICROPY_PY_LWIP Modified the register address area each chip in wiznet5k_regs function. When MICROPY_PY_LWIP is set as '0' , it can't read the register value of CREG in wiznet5k_regs function. Change the location of netinfo structure in the wiznet5k_init function.
ports/rp2 : change the blank code.
|
@dpgeorge |
# Conflicts: # extmod/network_wiznet5k.c
There was a problem hiding this comment.
Thanks @irinakim12
The code formatting is failing because it's confused about how the indentation of the preprocessor macros should work. I think this is a bug in how the code format tool works. Either way, if you make my suggested change then the problem will go away anyway.
The nRF build is failing for unrelated reasons. Looks like the soft device downloads are no longer available.
If you have a minute could you also please take a look at Wiznet/ioLibrary_Driver#120 (comment)
| @@ -766,7 +768,13 @@ STATIC mp_obj_t wiznet5k_make_new(const mp_obj_type_t *type, size_t n_args, size | |||
| STATIC mp_obj_t wiznet5k_regs(mp_obj_t self_in) { | |||
| (void)self_in; | |||
| printf("Wiz CREG:"); | |||
| #if _WIZCHIP_ == 5500 | |||
There was a problem hiding this comment.
Rather than duplicating the for loop 3 times, can you do
#if _WIZCHIP_ == 5500
const int nregs = 0x50;
#elif _WIZCHIP_ == 5105
const int nregs = 0x90;
#else
const int nregs = 0x60;
#endif
for (int i = 0; i < nregs; ++i) {
There was a problem hiding this comment.
Rather than having the #else be 0x60 -- can the default be the lower number, and explicitly set it to the higher number when it's known to be a chip with more registers?
| @@ -388,6 +379,17 @@ STATIC void wiznet5k_init(void) { | |||
| }; | |||
| wiznet5k_obj.netinfo = netinfo; | |||
|
|
|||
| // Configure wiznet provided TCP / socket interface | |||
There was a problem hiding this comment.
This change makes sense (calling ctlnetwork after actually initialising netinfo), but maybe only the ctlnetwork line needs to move?
There was a problem hiding this comment.
Right,
In conclusion, after initializing netinfo, call the ctlnetwork function and set the network-related register to the initialized value
| netutils_parse_ipv4_addr(items[2], netinfo.gw, NETUTILS_BIG); | ||
| netutils_parse_ipv4_addr(items[3], netinfo.dns, NETUTILS_BIG); | ||
| ctlnetwork(CN_SET_NETINFO, &netinfo); | ||
| netutils_parse_ipv4_addr(items[0], self->netinfo.ip, NETUTILS_BIG); |
There was a problem hiding this comment.
Can you explain this change. I think it makes sense, but it would be good to clarify -- is self->netinfo only for DHCP or is it correct to also put the static address into it? What goes wrong if you don't make this change?
| @@ -199,6 +199,10 @@ if (MICROPY_PY_LWIP) | |||
| target_compile_definitions(${MICROPY_TARGET} PRIVATE | |||
| MICROPY_PY_LWIP=1 | |||
| ) | |||
| else() | |||
| target_compile_definitions(${MICROPY_TARGET} PRIVATE | |||
| MICROPY_PY_LWIP=0 | |||
There was a problem hiding this comment.
This shouldn't be necessary. The default will be zero.
| @@ -6,9 +6,14 @@ BOARD ?= PICO | |||
|
|
|||
| BUILD ?= build-$(BOARD) | |||
|
|
|||
| LWIP ?= 0 | |||
There was a problem hiding this comment.
I don't think it makes sense to put this into Makefile. If a board wants to configure LWIP/WizNet then that should be done in mpconfigboard.cmake.
This is an unfortunate detail of the way the CMake system works, but I don't think adding each individual option to the Makefile is the right way to solve it.
| @@ -1,4 +1,3 @@ | |||
| # cmake file for Wiznet W5100S-EVB-Pico. | |||
| set(PICO_BOARD wiznet_w5100s_evb_pico) | |||
| set(MICROPY_PY_NETWORK_WIZNET5K W5100S) | |||
| set(MICROPY_PY_LWIP 1) | |||
There was a problem hiding this comment.
These should stay (unless there's some reason we want to stop using LWIP on these boards?) @andrewleech ?
|
If I look at the list of changed files in this PR, almost all of them are not related to the intended change. So something went wrong when making the PR. |
I will check the pull request!!! |
Signed-off-by: Eunkyoung Kim <irinakim1225@gmail.com>
# Conflicts: # docs/samd/pinout.rst
… dev1 Signed-off-by: Eunkyoung Kim <irinakim1225@gmail.com>
Signed-off-by: Eunkyoung Kim <irinakim1225@gmail.com>
…d-setter no need to support kwargs in a setter
Issue
To do
When MICROPY_PY_LWIP is set as '1' , it can read the register value of CREG in wiznet5k_regs function.
When MICROPY_PY_LWIP is set as '1' and DHCP use ,the operation of assigning IP is very slow.