Skip to content
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.

Sign up for GitHub

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

also sync segment name through udp #4482

Open
wants to merge 7 commits into
base: main
Choose a base branch
from
Open

also sync segment name through udp #4482

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
10a599c
also sync segment name through udp
Liliputech Jan 11, 2025
56804d7
add a new option "Segment name" in the sync settings
Liliputech Jan 14, 2025
12bbd20
fix JSON and checkbox now properly reflects current state
Liliputech Jan 30, 2025
19229a1
Merge remote-tracking branch 'blazoncek/parallel-I2S' into udp_name_sync
Liliputech Feb 4, 2025
712eee7
couple optimization after review
Liliputech Feb 4, 2025
84857de
Merge branch 'main' into udp_name_sync
Liliputech Feb 19, 2025
26081bf
Check if received segment name is not longer than allowed
Liliputech Feb 19, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions wled00/cfg.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -443,6 +443,7 @@ bool deserializeConfig(JsonObject doc, bool fromFS) {
CJSON(receiveNotificationPalette, if_sync_recv["pal"]);
CJSON(receiveGroups, if_sync_recv["grp"]);
CJSON(receiveSegmentOptions, if_sync_recv["seg"]);
CJSON(receiveSegmentName, if_sync_recv["sn"]);
CJSON(receiveSegmentBounds, if_sync_recv["sb"]);

JsonObject if_sync_send = if_sync[F("send")];
Expand Down Expand Up @@ -950,6 +951,7 @@ void serializeConfig() {
if_sync_recv["pal"] = receiveNotificationPalette;
if_sync_recv["grp"] = receiveGroups;
if_sync_recv["seg"] = receiveSegmentOptions;
if_sync_recv["sn"] = receiveSegmentName;
if_sync_recv["sb"] = receiveSegmentBounds;

JsonObject if_sync_send = if_sync.createNestedObject(F("send"));
Expand Down
4 changes: 2 additions & 2 deletions wled00/data/settings_sync.htm
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,7 @@ <h3>Sync groups</h3>
</table>
<h3>Receive</h3>
<nowrap><input type="checkbox" name="RB">Brightness,</nowrap> <nowrap><input type="checkbox" name="RC">Color,</nowrap> <nowrap><input type="checkbox" name="RX">Effects,</nowrap> <nowrap>and <input type="checkbox" name="RP">Palette</nowrap><br>
<input type="checkbox" name="SO"> Segment options, <input type="checkbox" name="SG"> bounds
<input type="checkbox" name="SO"> Segment options, <input type="checkbox" name="SN"> name, <input type="checkbox" name="SG"> bounds
<h3>Send</h3>
Enable Sync on start: <input type="checkbox" name="SS"><br>
Send notifications on direct change: <input type="checkbox" name="SD"><br>
Expand Down Expand Up @@ -241,4 +241,4 @@ <h3>Serial</h3>
<button type="button" onclick="B()">Back</button><button type="submit">Save</button>
</form>
</body>
</html>
</html>
1 change: 1 addition & 0 deletions wled00/set.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -379,6 +379,7 @@ void handleSettingsSet(AsyncWebServerRequest *request, byte subPage)
receiveNotificationEffects = request->hasArg(F("RX"));
receiveNotificationPalette = request->hasArg(F("RP"));
receiveSegmentOptions = request->hasArg(F("SO"));
receiveSegmentName = request->hasArg(F("SN"));
receiveSegmentBounds = request->hasArg(F("SG"));
sendNotifications = request->hasArg(F("SS"));
notifyDirect = request->hasArg(F("SD"));
Expand Down
31 changes: 20 additions & 11 deletions wled00/udp.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,9 @@
* UDP sync notifier / Realtime / Hyperion / TPM2.NET
*/

#define UDP_SEG_SIZE 36
#define UDP_SEG_SIZE (36+WLED_MAX_SEGNAME_LEN)
#define SEG_OFFSET (41)
#define WLEDPACKETSIZE (41+(MAX_NUM_SEGMENTS*UDP_SEG_SIZE)+0)
#define WLEDPACKETSIZE (SEG_OFFSET+(MAX_NUM_SEGMENTS*UDP_SEG_SIZE)+0)
#define UDP_IN_MAXSIZE 1472
#define PRESUMED_NETWORK_DELAY 3 //how many ms could it take on avg to reach the receiver? This will be added to transmitted times

Expand Down Expand Up @@ -143,6 +143,11 @@ void notify(byte callMode, bool followUp)
udpOut[33+ofs] = selseg.startY & 0xFF;
udpOut[34+ofs] = selseg.stopY >> 8; // ATM always 0 as Segment::stopY is 8-bit
udpOut[35+ofs] = selseg.stopY & 0xFF;

if(selseg.name){
strcpy((char *)&udpOut[36+ofs], selseg.name);
}

++s;
}

Expand All @@ -155,15 +160,11 @@ void notify(byte callMode, bool followUp)
// send global data
DEBUG_PRINTLN(F("ESP-NOW sending first packet."));
const size_t bufferSize = sizeof(buffer.data)/sizeof(uint8_t);
size_t packetSize = 41;
size_t s0 = 0;
size_t packetSize = SEG_OFFSET;
memcpy(buffer.data, udpOut, packetSize);
// stuff as many segments in first packet as possible (normally up to 5)
for (size_t i = 0; packetSize < bufferSize && i < s; i++) {
memcpy(buffer.data + packetSize, &udpOut[41+i*UDP_SEG_SIZE], UDP_SEG_SIZE);
packetSize += UDP_SEG_SIZE;
s0++;
}
// since we sync the name, only one segment can fit in the first packet
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a waste of packet space. Do not forget that the same packet is re-used in ESP-NOW sync.
ESP-NOW packet can only contain 250 bytes so "compression" of data is a must.

memcpy(buffer.data + packetSize, &udpOut[41+UDP_SEG_SIZE], UDP_SEG_SIZE);
size_t s0 = 1;
if (s > s0) buffer.noOfPackets += 1 + ((s - s0) * UDP_SEG_SIZE) / bufferSize; // set number of packets
auto err = quickEspNow.send(ESPNOW_BROADCAST_ADDRESS, reinterpret_cast<const uint8_t*>(&buffer), packetSize+3);
if (!err && s0 < s) {
Expand Down Expand Up @@ -267,7 +268,7 @@ static void parseNotifyPacket(const uint8_t *udpIn) {
}
size_t inactiveSegs = 0;
for (size_t i = 0; i < numSrcSegs && i < strip.getMaxSegments(); i++) {
unsigned ofs = 41 + i*udpIn[40]; //start of segment offset byte
unsigned ofs = SEG_OFFSET + i*udpIn[40]; //start of segment offset byte
unsigned id = udpIn[0 +ofs];
DEBUG_PRINTF_P(PSTR("UDP segment received: %u\n"), id);
if (id > strip.getSegmentsNum()) break;
Expand Down Expand Up @@ -296,6 +297,7 @@ static void parseNotifyPacket(const uint8_t *udpIn) {
uint16_t startY = version > 11 ? (udpIn[32+ofs] << 8 | udpIn[33+ofs]) : 0;
uint16_t stopY = version > 11 ? (udpIn[34+ofs] << 8 | udpIn[35+ofs]) : 1;
uint16_t offset = (udpIn[7+ofs] << 8 | udpIn[8+ofs]);

if (!receiveSegmentOptions) {
DEBUG_PRINTF_P(PSTR("Set segment w/o options: %d [%d,%d;%d,%d]\n"), id, (int)start, (int)stop, (int)startY, (int)stopY);
strip.suspend(); //should not be needed as UDP handling is not done in ISR callbacks but still added "just in case"
Expand Down Expand Up @@ -338,6 +340,13 @@ static void parseNotifyPacket(const uint8_t *udpIn) {
selseg.check1 = (udpIn[31+ofs]>>7) & 0x1;
}
}
if (receiveSegmentName) {
const char* name = (char *) &udpIn[36+ofs];
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can pass udpIn[...]directly to setName() no need to check for length or content.
You also do not need to clearName() prior to setName().

Also, indentation is off.

if (strlen(name) < WLED_MAX_SEGNAME_LEN) {
selseg.clearName();
selseg.setName(name);
}
}
if (receiveSegmentBounds) {
DEBUG_PRINTF_P(PSTR("Set segment w/ options: %d [%d,%d;%d,%d]\n"), id, (int)start, (int)stop, (int)startY, (int)stopY);
strip.suspend(); //should not be needed as UDP handling is not done in ISR callbacks but still added "just in case"
Expand Down
6 changes: 5 additions & 1 deletion wled00/wled.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -650,18 +650,20 @@ typedef class Receive {
bool Color : 1;
bool Effects : 1;
bool SegmentOptions : 1;
bool SegmentName : 1;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's why there is reserved. Use that if possible.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm sorry but i don't understand how I'm supposed to use reserved... to me it looks like it is a variable to hold the sum of all boolean define above, which is then not to be changed by hand?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You should have not "resolved" this as you did not address my comment.
reservedis a placeholder for future enhancement as this PR so replace it with SegmentNameoption.

bool SegmentBounds : 1;
bool Direct : 1;
bool Palette : 1;
uint8_t reserved : 1;
};
};
Receive(int i) { Options = i; }
Receive(bool b, bool c, bool e, bool sO, bool sB, bool p)
Receive(bool b, bool c, bool e, bool sO, bool sN, bool sB, bool p)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Normally one would add new additions at the end to increase compatibility. Just a personal opinion.

: Brightness(b)
, Color(c)
, Effects(e)
, SegmentOptions(sO)
, SegmentName(sN)
, SegmentBounds(sB)
, Palette(p)
{};
Expand Down Expand Up @@ -693,6 +695,7 @@ WLED_GLOBAL send_notification_t notifyG _INIT(0b00001111);
#define receiveNotificationEffects receiveN.Effects
#define receiveNotificationPalette receiveN.Palette
#define receiveSegmentOptions receiveN.SegmentOptions
#define receiveSegmentName receiveN.SegmentName
#define receiveSegmentBounds receiveN.SegmentBounds
#define receiveDirect receiveN.Direct
#define notifyDirect notifyG.Direct
Expand All @@ -705,6 +708,7 @@ WLED_GLOBAL bool receiveNotificationColor _INIT(true); // apply color
WLED_GLOBAL bool receiveNotificationEffects _INIT(true); // apply effects setup
WLED_GLOBAL bool receiveNotificationPalette _INIT(true); // apply palette
WLED_GLOBAL bool receiveSegmentOptions _INIT(false); // apply segment options
WLED_GLOBAL bool receiveSegmentName _INIT(false);
WLED_GLOBAL bool receiveSegmentBounds _INIT(false); // apply segment bounds (start, stop, offset)
WLED_GLOBAL bool receiveDirect _INIT(true); // receive UDP/Hyperion realtime
WLED_GLOBAL bool notifyDirect _INIT(false); // send notification if change via UI or HTTP API
Expand Down
1 change: 1 addition & 0 deletions wled00/xml.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -417,6 +417,7 @@ void getSettingsJS(byte subPage, Print& settingsScript)
printSetFormCheckbox(settingsScript,PSTR("RX"),receiveNotificationEffects);
printSetFormCheckbox(settingsScript,PSTR("RP"),receiveNotificationPalette);
printSetFormCheckbox(settingsScript,PSTR("SO"),receiveSegmentOptions);
printSetFormCheckbox(settingsScript,PSTR("SN"),receiveSegmentName);
printSetFormCheckbox(settingsScript,PSTR("SG"),receiveSegmentBounds);
printSetFormCheckbox(settingsScript,PSTR("SS"),sendNotifications);
printSetFormCheckbox(settingsScript,PSTR("SD"),notifyDirect);
Expand Down