Add a new option to set the local IP address#9
Open
jonasoberschweiber wants to merge 4 commits into
Open
Conversation
This commit adds a new option to explicitly set the local IP address instead of relying on autodetection. This is useful if you want to run the application inside of a Docker container, which will have a Docker-internal IP address instead of the external, device-visible one. The new option is called `network.local-ip`.
rumpeltux
requested changes
Apr 1, 2024
Owner
rumpeltux
left a comment
rumpeltux
left a comment
There was a problem hiding this comment.
Thanks! I have some minor comments, once addressed should be good to merge.
|
|
||
| void init_item_config(struct item_config *item_config, | ||
| const struct item_config *template, char *name) { | ||
| const struct item_config *template, char name[1024]) { |
Owner
There was a problem hiding this comment.
Please revert. I pushed another change that should solve this compiler error.
| goto out; | ||
| } | ||
| dev_config->timeout = var_uint; | ||
| } else if (sscanf((char *)buf, "network.local-ip %64s", var_str) == 1) { |
Owner
There was a problem hiding this comment.
You read 64 bytes but only strncpy 15 later. Should probably read 15 here at most, then we can strncpy 16 and have a guaranteed 0-byte termination.
(I also didn’t write the project initially, C is not a great choice…)
|
|
||
| rc = brother_conn_get_local_ip(conn, local_ip); | ||
| if (config->local_ip != NULL) { | ||
| strncpy(local_ip, config->local_ip, 15); |
| dev->ip); | ||
| brother_conn_get_local_ip(g_dev_handler.button_conn, ip); | ||
| if (dev->config->local_ip != NULL) { | ||
| strncpy(ip, dev->config->local_ip, 15); |
Author
|
Done. Could you recheck? |
|
I too look forward to see these changes merged. Currently we have to configure our docker container to use |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This commit adds a new option to explicitly set the local IP address instead of relying on autodetection. This is useful if you want to run the application inside of a Docker container, which will have a Docker-internal IP address instead of the external, device-visible one. The new option is called
network.local-ip.Without this option, brother-scand will manage to register functions at the device, but the device will not be able to reach brother-scand, because it will have received the registrations with an IP address it can't reach.
I'm using this in brother-scand-config to build a Docker container and have tested that it works locally. Not sure whether this is the best way to implement this -- my C is a little rusty.