Skip to content

STM32H5 Fixes - Fixes DHCP issue broken in PR #65#71

Open
dgarske wants to merge 1 commit intowolfSSL:masterfrom
dgarske:stm32h5_demo
Open

STM32H5 Fixes - Fixes DHCP issue broken in PR #65#71
dgarske wants to merge 1 commit intowolfSSL:masterfrom
dgarske:stm32h5_demo

Conversation

@dgarske
Copy link
Contributor

@dgarske dgarske commented Mar 6, 2026

No description provided.

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Fixes DHCP lease acquisition issues on STM32H5 by correcting socket local IP handling during DHCP request and improving STM32H563 bring-up diagnostics.

Changes:

  • Reset DHCP UDP socket local_ip after sending DHCP request so ACK matching follows the correct receive path.
  • Add UART diagnostics (MAC print, DHCP init/timeout logging, ETH stats dump) and shorten DHCP timeout.
  • Adjust DHCP polling delay/tick comments and update TLS client compile-time flag name in banner output.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.

File Description
src/wolfip.c Clears DHCP socket local_ip after request send to avoid premature “offered IP” binding affecting ACK handling.
src/port/stm32h563/main.c Adds UART helpers and richer DHCP/ETH diagnostics; tweaks polling delay/timeout and TLS client banner guard.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

You can also share your feedback on Copilot code review. Take the survey.

Comment on lines +4992 to +4995
/* Reset local_ip so DHCP ACK matches via DHCP_IS_RUNNING path in
* udp_try_recv(). wolfIP_sock_sendto() sets local_ip from conf->ip
* (the offered IP), but we haven't confirmed the lease yet. */
s->udpsockets[SOCKET_UNMARK(s->dhcp_udp_sd)].local_ip = 0;
Copy link

Copilot AI Mar 6, 2026

Choose a reason for hiding this comment

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

This directly mutates internal socket state (s->udpsockets[...]) from DHCP code, which tightly couples DHCP behavior to socket internals. Consider encapsulating this in a small helper (e.g., a socket API to clear/reset local_ip for a given fd) or adjusting the wolfIP_sock_sendto() behavior for DHCP sockets so DHCP doesn’t need to reach into udpsockets internals.

Copilot uses AI. Check for mistakes.
dhcp_ret = dhcp_client_init(IPStack);
if (dhcp_ret < 0) {
uart_puts(" DHCP init failed (");
uart_puthex((uint32_t)dhcp_ret);
Copy link

Copilot AI Mar 6, 2026

Choose a reason for hiding this comment

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

Casting a negative dhcp_ret to uint32_t and printing as hex will produce an opaque two’s-complement value, which makes the error hard to interpret. Prefer printing a signed decimal code (or printing a leading '-' and the absolute value) so the UART log reflects the actual return code.

Suggested change
uart_puthex((uint32_t)dhcp_ret);
if (dhcp_ret < 0) {
uart_puts("-");
uart_puthex((uint32_t)(-dhcp_ret));
} else {
uart_puthex((uint32_t)dhcp_ret);
}

Copilot uses AI. Check for mistakes.
Comment on lines +611 to +614
/* Increment tick counter (approximate 1ms per iteration at 64MHz HSI)
* volatile loop ~8 cycles/iter: 8000 * 8 / 64MHz = 1ms */
tick++;
/* Small delay to allow Ethernet DMA to work */
delay(1000);
delay(8000);
Copy link

Copilot AI Mar 6, 2026

Choose a reason for hiding this comment

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

The loop assumes a specific core clock (64MHz HSI) and a specific delay() loop cost, but tick is still incremented by 1 regardless of the actual elapsed time, making DHCP timeout behavior fragile across clock configuration changes/optimizations. Consider driving the timeout from a real timebase (SysTick/HAL tick/monotonic timer) or deriving the delay count from SystemCoreClock and updating tick based on measured/actual elapsed time.

Copilot uses AI. Check for mistakes.
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.

3 participants