no-OS icon indicating copy to clipboard operation
no-OS copied to clipboard

Fix MAX32690APARD TCP Example Static IP Configuration

Open Brandon-Hurst opened this issue 1 year ago • 2 comments

Pull Request Description

Fix for Issue #2172 . Static IP configuration for MAX32690APARD example does not function as described in the documentation. Reason is due to some mishandling of Makefile scripts and preprocessor macros for IP addresses. Fix requires no code changes, only Makefile + README changes.

PR Type

  • [x] Bug fix (change that fixes an issue)
  • [ ] New feature (change that adds new functionality)
  • [ ] Breaking change (has dependencies in other repos or will cause CI to fail)

PR Checklist

  • [x] I have followed the Coding style guidelines
  • [x] I have performed a self-review of the changes
  • [x] I have commented my code, at least hard-to-understand parts
  • [x] I have build all projects affected by the changes in this PR
  • [x] I have tested in hardware affected projects, at the relevant boards
  • [x] I have signed off all commits from this PR
  • [x] I have updated the documentation (wiki pages, ReadMe etc), if applies

Brandon-Hurst avatar May 03 '24 20:05 Brandon-Hurst

CLA assistant check
All committers have signed the CLA.

CLAassistant avatar May 03 '24 20:05 CLAassistant

Hi @Brandon-Hurst,

Thanks for reporting this issue and submitting a fix. There is already a similar way of defining the NO_OS_IP, NO_OS_NETMASK and NO_OS_GATEWAY macros in this file https://github.com/analogdevicesinc/no-OS/blob/main/tools/scripts/lwip.mk#L21-L31 . However, the real issue is that the makefile variables with the same name are defined after lwip.mk is included, thus they are ignored resulting in a DHCP request instead of just assigning the static IP.

I've submitted a similar PR which implements the fix here https://github.com/analogdevicesinc/no-OS/pull/2176 . However, as I've changed the IP, there is still need to write some document explaining how to assign static IPs on Linux and Windows (as you pointed out in your github issue).

Let me know if besides this, we should modify the documentation.

CiprianRegus avatar May 07 '24 11:05 CiprianRegus

addressed in #2176

buha avatar May 13 '24 12:05 buha

Yep, I tested on my side. Thanks for updating the docs as well, the GUI form shown on the new wiki page looks okay. I tested that and it worked alright. I was using the Command Prompt as admin on Windows and the "route ADD" command to add the static route for the IP address. Thanks for the quick fixes!

Brandon-Hurst avatar May 13 '24 22:05 Brandon-Hurst