Fix MAX32690APARD TCP Example Static IP Configuration

analogdevicesinc / no-OS

Software drivers in C for systems without an operating system

http://analogdevicesinc.github.io/no-OS/

Other

924 stars 1.65k forks source link

Fix MAX32690APARD TCP Example Static IP Configuration #2173

Closed Brandon-Hurst closed 4 months ago

Brandon-Hurst commented 5 months ago

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

CLAassistant commented 5 months ago

All committers have signed the CLA.

CiprianRegus commented 5 months ago

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.

buha commented 4 months ago

addressed in #2176

Brandon-Hurst commented 4 months ago

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!