-
-
Notifications
You must be signed in to change notification settings - Fork 3.9k
Add mDNS resolution for network bus #4768
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -133,6 +133,7 @@ class Bus { | |
| virtual uint16_t getUsedCurrent() const { return 0; } | ||
| virtual uint16_t getMaxCurrent() const { return 0; } | ||
| virtual size_t getBusSize() const { return sizeof(Bus); } | ||
| virtual const String getCustomText() const { return String(); } | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Two ideas for improvement:
|
||
|
|
||
| inline bool hasRGB() const { return _hasRgb; } | ||
| inline bool hasWhite() const { return _hasWhite; } | ||
|
|
@@ -215,7 +216,7 @@ class Bus { | |
| uint8_t _autoWhiteMode; | ||
| // global Auto White Calculation override | ||
| static uint8_t _gAWM; | ||
| // _cct has the following menaings (see calculateCCT() & BusManager::setSegmentCCT()): | ||
| // _cct has the following meanings (see calculateCCT() & BusManager::setSegmentCCT()): | ||
| // -1 means to extract approximate CCT value in K from RGB (in calcualteCCT()) | ||
| // [0,255] is the exact CCT value where 0 means warm and 255 cold | ||
| // [1900,10060] only for color correction expressed in K (colorBalanceFromKelvin()) | ||
|
|
@@ -342,6 +343,10 @@ class BusNetwork : public Bus { | |
| size_t getBusSize() const override { return sizeof(BusNetwork) + (isOk() ? _len * _UDPchannels : 0); } | ||
| void show() override; | ||
| void cleanup(); | ||
| #ifdef ARDUINO_ARCH_ESP32 | ||
| void resolveHostname(); | ||
| const String getCustomText() const override { return _hostname; } | ||
| #endif | ||
|
|
||
| static std::vector<LEDType> getLEDTypes(); | ||
|
|
||
|
|
@@ -351,6 +356,9 @@ class BusNetwork : public Bus { | |
| uint8_t _UDPchannels; | ||
| bool _broadcastLock; | ||
| uint8_t *_data; | ||
| #ifdef ARDUINO_ARCH_ESP32 | ||
| String _hostname; | ||
| #endif | ||
| }; | ||
|
|
||
|
|
||
|
|
@@ -368,8 +376,9 @@ struct BusConfig { | |
| uint16_t frequency; | ||
| uint8_t milliAmpsPerLed; | ||
| uint16_t milliAmpsMax; | ||
| String text; | ||
|
|
||
| BusConfig(uint8_t busType, uint8_t* ppins, uint16_t pstart, uint16_t len = 1, uint8_t pcolorOrder = COL_ORDER_GRB, bool rev = false, uint8_t skip = 0, byte aw=RGBW_MODE_MANUAL_ONLY, uint16_t clock_kHz=0U, uint8_t maPerLed=LED_MILLIAMPS_DEFAULT, uint16_t maMax=ABL_MILLIAMPS_DEFAULT) | ||
| BusConfig(uint8_t busType, uint8_t* ppins, uint16_t pstart, uint16_t len = 1, uint8_t pcolorOrder = COL_ORDER_GRB, bool rev = false, uint8_t skip = 0, byte aw=RGBW_MODE_MANUAL_ONLY, uint16_t clock_kHz=0U, uint8_t maPerLed=LED_MILLIAMPS_DEFAULT, uint16_t maMax=ABL_MILLIAMPS_DEFAULT, String sometext = "") | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. some text? I thought this is about an mDNS name. |
||
| : count(std::max(len,(uint16_t)1)) | ||
| , start(pstart) | ||
| , colorOrder(pcolorOrder) | ||
|
|
@@ -379,6 +388,7 @@ struct BusConfig { | |
| , frequency(clock_kHz) | ||
| , milliAmpsPerLed(maPerLed) | ||
| , milliAmpsMax(maMax) | ||
| , text(sometext) | ||
| { | ||
| refreshReq = (bool) GET_BIT(busType,7); | ||
| type = busType & 0x7F; // bit 7 may be/is hacked to include refresh info (1=refresh in off state, 0=no refresh) | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -52,6 +52,11 @@ | |
| maxL = l; // maxL - max LEDs (will serve to determine ESP >1664 == ESP32) | ||
| maxCO = o; // maxCO - max Color Order mappings | ||
| } | ||
| function is8266() { return maxA == 5 && maxD == 3; } // NOTE: see const.h | ||
| function is32() { return maxA == 16 && maxD == 16; } // NOTE: see const.h | ||
| function isC3() { return maxA == 6 && maxD == 2; } // NOTE: see const.h | ||
| function isS2() { return maxA == 8 && maxD == 12 && maxV == 4; } // NOTE: see const.h | ||
| function isS3() { return maxA == 8 && maxD == 12 && maxV == 6; } // NOTE: see const.h | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This looks very fragile. Especially if someone changes the number of virtual buses by overriding the value in const.h, then "isS3" will stop working. |
||
| function pinsOK() { | ||
| var ok = true; | ||
| var nList = d.Sf.querySelectorAll("#mLC input[name^=L]"); | ||
|
|
@@ -271,7 +276,7 @@ | |
| gRGBW |= hasW(t); // RGBW checkbox | ||
| gId("co"+n).style.display = (isVir(t) || isAna(t)) ? "none":"inline"; // hide color order for PWM | ||
| gId("dig"+n+"w").style.display = (isDig(t) && hasW(t)) ? "inline":"none"; // show swap channels dropdown | ||
| gId("dig"+n+"w").querySelector("[data-opt=CCT]").disabled = !hasCCT(t); // disable WW/CW swapping | ||
| gId("dig"+n+"w").querySelector("[data-opt=CCT]").disabled = !hasCCT(t); // disable WW/CW swapping | ||
| if (!(isDig(t) && hasW(t))) d.Sf["WO"+n].value = 0; // reset swapping | ||
| gId("dig"+n+"c").style.display = (isAna(t)) ? "none":"inline"; // hide count for analog | ||
| gId("dig"+n+"r").style.display = (isVir(t)) ? "none":"inline"; // hide reversed for virtual | ||
|
|
@@ -281,6 +286,8 @@ | |
| gId("dig"+n+"l").style.display = (isD2P(t) || isPWM(t)) ? "inline":"none"; // bus clock speed / PWM speed (relative) (not On/Off) | ||
| gId("rev"+n).innerHTML = isAna(t) ? "Inverted output":"Reversed"; // change reverse text for analog else (rotated 180°) | ||
| //gId("psd"+n).innerHTML = isAna(t) ? "Index:":"Start:"; // change analog start description | ||
| gId("net"+n+"h").style.display = isNet(t) && !is8266() ? "block" : "none"; // show host field for network types except on ESP8266 | ||
| if (!isNet(t) || is8266()) d.Sf["HS"+n].value = ""; // cleart host field if not network type or ESP8266 | ||
| }); | ||
| // display global white channel overrides | ||
| gId("wc").style.display = (gRGBW) ? 'inline':'none'; | ||
|
|
@@ -463,6 +470,7 @@ | |
| <span id="p2d${s}"></span><input type="number" name="L2${s}" class="s" onchange="UI();pinUpd(this);"/> | ||
| <span id="p3d${s}"></span><input type="number" name="L3${s}" class="s" onchange="UI();pinUpd(this);"/> | ||
| <span id="p4d${s}"></span><input type="number" name="L4${s}" class="s" onchange="UI();pinUpd(this);"/> | ||
| <div id="net${s}h" class="hide">Host: <input type="text" name="HS${s}" maxlength="32" pattern="[a-zA-Z0-9_\\-]*" onchange="UI()"/>.local</div> | ||
| <div id="dig${s}r" style="display:inline"><br><span id="rev${s}">Reversed</span>: <input type="checkbox" name="CV${s}"></div> | ||
| <div id="dig${s}s" style="display:inline"><br>Skip first LEDs: <input type="number" name="SL${s}" min="0" max="255" value="0" oninput="UI()"></div> | ||
| <div id="dig${s}f" style="display:inline"><br><span id="off${s}">Off Refresh</span>: <input id="rf${s}" type="checkbox" name="RF${s}"></div> | ||
|
|
@@ -479,7 +487,7 @@ | |
| if (type.t != undefined && type.t != "") { | ||
| opt.setAttribute('data-type', type.t); | ||
| } | ||
| sel.appendChild(opt); | ||
| sel.appendChild(opt); | ||
| } | ||
| } | ||
| }); | ||
|
|
@@ -586,7 +594,7 @@ | |
| var cs = false; | ||
| for (var i=1; i < gEBCN("iST").length; i++) { | ||
| var s = chrID(i); | ||
| var p = chrID(i-1); // cover edge case 'A' previous char being '9' | ||
| var p = chrID(i-1); // cover edge case 'A' previous char being '9' | ||
| var v = parseInt(gId("ls"+p).value) + parseInt(gN("LC"+p).value); | ||
| if (v != parseInt(gId("ls"+s).value)) {cs = true; startsDirty[i] = true;} | ||
| } | ||
|
|
@@ -617,7 +625,7 @@ | |
|
|
||
| function receivedText(e) { | ||
| let lines = e.target.result; | ||
| let c = JSON.parse(lines); | ||
| let c = JSON.parse(lines); | ||
| if (c.hw) { | ||
| if (c.hw.led) { | ||
| // remove all existing outputs | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Implement error handling for hostname resolution failures.
The resolveHostname() method has good throttling (10-minute intervals) but could benefit from improved error handling and logging.
Consider adding error handling and debug logging:
void BusNetwork::resolveHostname() { static unsigned long nextResolve = 0; if (Network.isConnected() && millis() > nextResolve && _hostname.length() > 0) { nextResolve = millis() + 600000; // resolve only every 10 minutes IPAddress clnt; + bool resolved = false; if (strlen(cmDNS) > 0) clnt = MDNS.queryHost(_hostname); + if (clnt != IPAddress()) resolved = true; else WiFi.hostByName(_hostname.c_str(), clnt); + if (!resolved && clnt != IPAddress()) resolved = true; - if (clnt != IPAddress()) _client = clnt; + if (resolved) { + _client = clnt; + DEBUGBUS_PRINTF_P(PSTR("Bus: Resolved hostname %s to %s\n"), _hostname.c_str(), _client.toString().c_str()); + } else { + DEBUGBUS_PRINTF_P(PSTR("Bus: Failed to resolve hostname %s\n"), _hostname.c_str()); + } } }🤖 Prompt for AI Agents