8000 ports/rp2: Change MICROPY_PY_LWIP setting value and netinfo structure. by irinakim12 · Pull Request #9862 · micropython/micropython · GitHub
[go: up one dir, main page]
Skip to content

ports/rp2: Change MICROPY_PY_LWIP setting value and netinfo structure. #9862

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.

Sign up for GitHub

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

Closed
wants to merge 23 commits into from
Closed

ports/rp2: Change MICROPY_PY_LWIP setting value and netinfo structure. #9862

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
80c17ef
- Modified the register address area each chip in wiznet5k_regs fun…
irinakim12 Oct 18, 2022
d93a724
Deleted unnecessary comment
irinakim12 Oct 18, 2022
e1094c3
Changed code formatting
irinakim12 Oct 18, 2022
8612af1
Deleted whitespace
irinakim12 Oct 18, 2022
693de0e
Deleted whitespace
irinakim12 Oct 18, 2022
d11ef62
ports/rp2: Add MICROPY_PY_LWIP setting value.
irinakim12 Oct 18, 2022
3e8daad
Merge branch 'dev'
irinakim12 Oct 18, 2022
85bf7ab
ports/rp2: Add MICROPY_PY_LWIP setting value.
irinakim12 Oct 18, 2022
7f50ef9
Merge branch 'master' of https://github.com/irinakim12/micropython
irinakim12 Oct 24, 2022
5aa9829
ports/rp2: Change MICROPY_PY_LWIP setting value and netinfo structure.
irinakim12 Nov 4, 2022
22c4973
Update network_wiznet5k.c
irinakim12 Nov 4, 2022
3d3e789
Merge branch 'dev1'
irinakim12 Nov 7, 2022
0ad7506
ports/rp2: Change the register value for each chip
irinakim12 Nov 11, 2022
c120d11
Merge remote-tracking branch 'upstream/master' into dev1
irinakim12 Nov 11, 2022
fae5335
extmod/mbedtls: it was accidentally deleted during commit process.
irinakim12 Nov 11, 2022
42100f0
extmod/mbedtls: Add folder that were deleted during the commit.
irinakim12 Nov 11, 2022
22e8c8d
Merge branch 'dev1' of https://github.com/irinakim12/micropython into…
irinakim12 Nov 11, 2022
613da47
Merge branch 'micropython:master' into dev1
irinakim12 Nov 11, 2022
7eedac7
Merge branch 'dev1' of https://github.com/irinakim12/micropython into…
irinakim12 Nov 11, 2022
4038c39
Merge branch 'dev1' of https://github.com/irinakim12/micropython into…
irinakim12 Nov 11, 2022
0ed85af
ports/rp2: Delete the comment
irinakim12 Nov 11, 2022
37baba7
ports/rp2: Change the register value for each chip.
irinakim12 Nov 14, 2022
0769ab8
Merge branch 'dev1' of https://github.com/irinakim12/micropython into…
irinakim12 Nov 14, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
36 changes: 22 additions & 14 deletions extmod/network_wiznet5k.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -368,15 +368,6 @@ STATIC void wiz_dhcp_conflict(void) {
}

STATIC void wiznet5k_init(void) {
// Configure wiznet provided TCP / socket interface

reg_dhcp_cbfunc(wiz_dhcp_assign, wiz_dhcp_update, wiz_dhcp_conflict);

uint8_t sn_size[16] = {2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2}; // 2k buffer for each socket
ctlwizchip(CW_INIT_WIZCHIP, sn_size);

ctlnetwork(CN_SET_NETINFO, (void *)&wiznet5k_obj.netinfo);

// set some sensible default values; they are configurable using ifconfig method
wiz_NetInfo netinfo = {
.mac = {0, 0, 0, 0, 0, 0},
Expand All @@ -388,6 +379,17 @@ STATIC void wiznet5k_init(void) {
};
wiznet5k_obj.netinfo = netinfo;

// Configure wiznet provided TCP / socket interface
Copy link
Member

Choose a reason for hiding this comment

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

This change makes sense (calling ctlnetwork after actually initialising netinfo), but maybe only the ctlnetwork line needs to move?

Copy link
Author

Choose a reason for hiding this comment

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

Right,
In conclusion, after initializing netinfo, call the ctlnetwork function and set the network-related register to the initialized value


reg_dhcp_cbfunc(wiz_dhcp_assign, wiz_dhcp_update, wiz_dhcp_conflict);

uint8_t sn_size[16] = {2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2}; // 2k buffer for each socket
ctlwizchip(CW_INIT_WIZCHIP, sn_size);

ctlnetwork(CN_SET_NETINFO, (void *)&wiznet5k_obj.netinfo);



// register with network module
mod_network_register_nic(&wiznet5k_obj);

Expand Down Expand Up @@ -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
Copy link
Member

Choose a reason for hiding this comment

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

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) {

Copy link
Member

Choose a reason for hiding this comment

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

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?

for (int i = 0; i < 0x50; ++i) {
#elif _WIZCHIP_ == 5105
for (int i = 0; i < 0x90; ++i) {
#else
for (int i = 0; i < 0x60; ++i) {
#endif
if (i % 16 == 0) {
printf("\n %04x:", i);
}
Expand Down Expand Up @@ -896,11 +904,11 @@ STATIC mp_obj_t wiznet5k_ifconfig(size_t n_args, const mp_obj_t *args) {
self->netinfo.dhcp = NETINFO_STATIC;
mp_obj_t *items;
mp_obj_get_array_fixed_n(args[1], 4, &items);
netutils_parse_ipv4_addr(items[0], netinfo.ip, NETUTILS_BIG);
netutils_parse_ipv4_addr(items[1], netinfo.sn, NETUTILS_BIG);
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);
Copy link
Member

Choose a reason for hiding this comment

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

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?

netutils_parse_ipv4_addr(items[1], self->netinfo.sn, NETUTILS_BIG);
netutils_parse_ipv4_addr(items[2], self->netinfo.gw, NETUTILS_BIG);
netutils_parse_ipv4_addr(items[3], self->netinfo.dns, NETUTILS_BIG);
ctlnetwork(CN_SET_NETINFO, &self->netinfo);
return mp_const_none;
}
}
Expand Down
4 changes: 4 additions & 0 deletions ports/rp2/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Copy link
Member

Choose a reason for hiding this comment

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

This shouldn't be necessary. The default will be zero.

)
endif()

if(MICROPY_PY_BLUETOOTH)
Expand Down
6 changes: 6 additions & 0 deletions ports/rp2/Makefile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,14 @@ BOARD ?= PICO

BUILD ?= build-$(BOARD)

LWIP ?= 0
Copy link
Member

Choose a reason for hiding this comment

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

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.


MICROPY_PY_WIZNET5K ?= 5105

$(VERBOSE)MAKESILENT = -s

CMAKE_ARGS = -DMICROPY_BOARD=$(BOARD)
CMAKE_ARGS += -DMICROPY_PY_WIZNET5K=$(MICROPY_PY_WIZNET5K)

ifdef USER_C_MODULES
CMAKE_ARGS += -DUSER_C_MODULES=${USER_C_MODULES}
Expand All @@ -26,6 +31,7 @@ ifdef BOARD_VARIANT
CMAKE_ARGS += -DBOARD_VARIANT=${BOARD_VARIANT}
endif

CMAKE_ARGS += -DMICROPY_PY_LWIP=${LWIP}
HELP_BUILD_ERROR ?= "See \033[1;31mhttps://github.com/micropython/micropython/wiki/Build-Troubleshooting\033[0m"

all:
Expand Down
1 change: 0 additions & 1 deletion ports/rp2/boards/W5100S_EVB_PICO/mpconfigboard.cmake
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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)
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 stay (unless there's some reason we want to stop using LWIP on these boards?) @andrewleech ?

1 change: 0 additions & 1 deletion ports/rp2/boards/W5500_EVB_PICO/mpconfigboard.cmake
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
# cmake file for Wiznet W5500-EVB-Pico.
set(PICO_BOARD wiznet_w5100s_evb_pico)
set(MICROPY_PY_NETWORK_WIZNET5K W5500)
set(MICROPY_PY_LWIP 1)
0