Skip to content
This repository has been archived by the owner on Feb 8, 2024. It is now read-only.

Add optional device hostName setting #140

Closed
wants to merge 8 commits into from
Closed

Conversation

aaronse
Copy link

@aaronse aaronse commented Jan 18, 2022

problem: Various projects using this framework, deployed to various devices appear on networks as ESP-<MAC Address last 3 bytes hex encoded> e.g. ESP-214365.
cause: This framework isn't setting hostName before calling Wifi.begin(), or exposing hostName as a configuration setting.
fix: Add optional device hostName setting. Edits are intended to not break, or change behavior of existing projects. Intention is that Devs can opt in to using if/when needed.

@maakbaas
Copy link
Owner

Thanks for proposing and adding this!

I am currently away from home, so there will be a delay in merging this to the repository.

@maakbaas
Copy link
Owner

Can you look into why the automated build failed?

This reverts commit 9eaeb55.
@aaronse
Copy link
Author

aaronse commented Jan 26, 2022

Looking...

@aaronse
Copy link
Author

aaronse commented Jan 27, 2022

Ok, finally figured out how to repro example configManager build failure on my local wintel dev box. Do the docs have, or should they have, a FAQ or something like this...

How to locally repro and troubleshoot /examples/... build/test failure(s)

  • Start PlatformIO Core CLI shell from within VSCode.
  • Copy paste build params from travis.yml or build output, appending relative folder path for the example being investigated, e.g. for configManager do..
$ platformio ci --lib="." --board=nodemcuv2 --project-option="lib_deps=https://github.com/lorol/ESPAsyncWebServer, ArduinoJSON" --project-option="build_flags = -DCONFIG_PATH=src/configuration.json 
-DREBUILD_CONFIG" **examples/configManager/**

still need to understand and fix...

@aaronse
Copy link
Author

aaronse commented Jan 27, 2022

On my WinTel, npm run lint fails with 2000+, CRLF errors, drowning out ability to see real failures.

  error  Expected linebreaks to be 'LF' but found 'CRLF'  linebreak-style

Am editing .eslintrc.js ...

        "linebreak-style": [
            "error",
            "unix",
        ],

To instead be sensitive to platform build is executing on...

        "linebreak-style": [
            "error",
            (require("os").EOL === "\r\n" ? "windows" : "unix"),
        ],

@aaronse
Copy link
Author

aaronse commented Feb 3, 2022

@maakbaas, I appreciate you might be travelling/unavailable. When ever you're available next, let me know whether/what additional edits are needed for this pull request. Cheers!

@maakbaas
Copy link
Owner

maakbaas commented Feb 3, 2022

Will do, sorry about the delay. I am actually traveling till mid march, so it will still be a bit longer.

@maakbaas
Copy link
Owner

Hi @aaronse. Thanks for the pull request and the idea to add hostname is fine. I really appreciate your efforts. However, again like #44, this pull request contains many small changes which are unrelated to this, and should not all be part of the framework. This makes it hard for me to commit it.

I understand that you are doing your own development, but I can only work with pull requests that are based on the current master and contain a single change.

@maakbaas maakbaas closed this Apr 24, 2022
@aaronse
Copy link
Author

aaronse commented Apr 24, 2022

Hey @maakbaas, hope you're well. Will try again with some of the edits split into separate pull requests.

@maakbaas maakbaas mentioned this pull request Apr 24, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants