Skip to content

Commit 0445b14

Browse files
committed
fix(Comm): increase json buffer size to handle large thumbnail responses from DSF
1 parent bb507f0 commit 0445b14

File tree

6 files changed

+195
-7
lines changed

6 files changed

+195
-7
lines changed

src/Comm/JsonDecoder.cpp

Lines changed: 22 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -464,7 +464,27 @@ namespace Comm
464464
}
465465
}
466466

467-
// Look for combining characters in the string value and convert them if possible
467+
/**
468+
* @brief Convert Unicode combining characters in the string value to their precomposed equivalents if possible.
469+
*
470+
* Some text editors save accented characters as a combination of a base character and a combining diacritical mark.
471+
* For example, "á" might be saved as "a" followed by a combining acute accent. This function looks for such
472+
* combinations in the string value and converts them to their precomposed equivalents if possible. This is done by
473+
* looking for UTF-8 encoded Unicode characters in the string value and checking if they are combining diacritical
474+
* marks that we handle. If they are, we look at the previous character and see if it can be combined with the
475+
* diacritical mark to form a precomposed character. If it can, we replace the previous character with the
476+
* precomposed character and remove the diacritical mark from the string.
477+
*
478+
* @note This function is causes significant slow downs when processing large JSON responses (SBC thumbnails). It is
479+
* currently disabled since LVGL has some limited support for rendering multi codepoint graphemes, and the main
480+
* issue with not converting is that some diacritical marks are rendered as separate characters instead of being
481+
* combined with the previous character. If this becomes a bigger issue, we could consider only converting certain
482+
* common combinations of characters and diacritical marks, or we could look into optimizing the function to reduce
483+
* the performance impact.
484+
*
485+
* @note Since the Json parser is due to be reworked anyway. It will be disabled until then unless there is an
486+
* explicit complaint.
487+
*/
468488
void JsonDecoder::ConvertUnicode()
469489
{
470490
ZoneScoped;
@@ -852,7 +872,7 @@ namespace Comm
852872
switch (c)
853873
{
854874
case '"':
855-
ConvertUnicode();
875+
// ConvertUnicode();
856876
ProcessField();
857877
m_state = jsEndVal;
858878
break;

src/Comm/Thumbnail.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -335,7 +335,7 @@ bool DeleteCachedThumbnail(std::string_view filepath)
335335
{
336336
LOG_INFO("Deleting thumbnail for {:s}", filepath);
337337
bool ret = true;
338-
ret &= std::filesystem::remove(GetThumbnailPath(filepath, false));
338+
// ret &= std::filesystem::remove(GetThumbnailPath(filepath, false));
339339
ret &= std::filesystem::remove(GetThumbnailPath(filepath, true));
340340
return ret;
341341
}

src/Configuration.h

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,7 @@ constexpr std::string_view DEFAULT_FILAMENTS_FILE = "filaments.csv";
5757
constexpr std::string_view DEFAULT_HEIGHTMAPS_FILE = "heightmaps.csv";
5858

5959
/* Thumbnails */
60-
constexpr std::chrono::milliseconds FILE_CACHE_REQUEST_TIMEOUT = 5000ms;
60+
constexpr std::chrono::milliseconds FILE_CACHE_REQUEST_TIMEOUT = 10000ms;
6161
constexpr size_t MAX_THUMBNAIL_CACHE_PIXELS = 64; // Largest pixel width/height thumbnail that is allowed to be cached
6262
constexpr std::chrono::milliseconds BACKGROUND_FILE_CACHE_POLL_INTERVAL = 500ms;
6363
constexpr size_t MAX_FILEINFO_REQUESTS = 1;
@@ -68,7 +68,7 @@ constexpr size_t MAX_ARRAY_NESTING = 4;
6868
constexpr size_t MAX_JSON_ID_LENGTH = 200;
6969
// 4096 is the largest needed for a Duet in standalone mode. But in
7070
// SBC mode, network responses can be much larger. This is most evident with `rr_thumbnail`
71-
constexpr size_t MAX_JSON_VALUE_LENGTH = 4096 * 5;
71+
constexpr size_t MAX_JSON_VALUE_LENGTH = 4096 * 20;
7272

7373
/* Network */
7474
// Duet 2 seems to only support 3 concurrent connections. We need 1 connection for synchronous requests, so we can

src/image/png.cpp

Lines changed: 41 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,9 @@
22

33
#include "png.h"
44
#include <algorithm>
5+
#include <errno.h>
56
#include <stdio.h>
7+
#include <unistd.h>
68

79
PNG::PNG()
810
: m_imageFileName(nullptr)
@@ -46,6 +48,22 @@ bool PNG::Close()
4648
return true;
4749
}
4850
LOG_DBG("Closing file {:s}", m_imageFileName);
51+
if (fflush(m_imageFile) != 0)
52+
{
53+
LOG_ERROR("Failed to fflush file {:s}: {}", m_imageFileName, errno);
54+
/* attempt to continue to close */
55+
}
56+
57+
int fd = fileno(m_imageFile);
58+
if (fd != -1)
59+
{
60+
if (fsync(fd) != 0)
61+
{
62+
LOG_ERROR("Failed to fsync file {:s}: {}", m_imageFileName, errno);
63+
/* attempt to continue to close */
64+
}
65+
}
66+
4967
if (fclose(m_imageFile) != 0)
5068
{
5169
LOG_ERROR("Failed to close file {:s}", m_imageFileName);
@@ -62,6 +80,27 @@ size_t PNG::appendData(unsigned char data[], int size)
6280
LOG_WARN("File {:s} not open", m_imageFileName);
6381
return 0;
6482
}
65-
dataSize += size;
66-
return fwrite(data, 1, size, m_imageFile);
83+
if (size <= 0)
84+
{
85+
return 0;
86+
}
87+
88+
size_t totalWritten = 0;
89+
while (totalWritten < static_cast<size_t>(size))
90+
{
91+
size_t toWrite = static_cast<size_t>(size) - totalWritten;
92+
size_t written = fwrite(data + totalWritten, 1, toWrite, m_imageFile);
93+
if (written == 0)
94+
{
95+
if (ferror(m_imageFile))
96+
{
97+
LOG_ERROR("Failed to write PNG data to {:s}: {}", m_imageFileName, errno);
98+
}
99+
break;
100+
}
101+
totalWritten += written;
102+
}
103+
104+
dataSize += totalWritten;
105+
return totalWritten;
67106
}
3.55 KB
Loading
Lines changed: 129 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,129 @@
1+
/*
2+
* test_diacritic.cpp
3+
*
4+
* Created on: 2026-02-23
5+
* Author: automated-test
6+
*/
7+
8+
#include "Debug.h"
9+
#include "UI/Components/LVGL/LvLabel.h"
10+
#include "test_utils/UiTestSuite.h"
11+
#include <cstring>
12+
#include <gtest/gtest.h>
13+
#include <string>
14+
15+
using namespace UI;
16+
17+
class TestDiacritic : public UiTestSuite
18+
{
19+
public:
20+
TestDiacritic() {}
21+
};
22+
23+
TEST_F(TestDiacritic, CombinedVsCombining)
24+
{
25+
// Create two labels on the test screen
26+
LvLabel label_precomposed("label_pre", screen);
27+
LvLabel label_decomposed("label_decomp", screen);
28+
29+
// Examples: pairs of (precomposed, decomposed)
30+
// This list covers common diacritics used across European languages.
31+
struct Pair
32+
{
33+
const char* pre;
34+
const char* decomp;
35+
const char* desc;
36+
} pairs[] = {
37+
{"\u00E1", "a\u0301", "a + acute -> á"}, // á
38+
{"\u00E0", "a\u0300", "a + grave -> à"}, // à
39+
{"\u00E2", "a\u0302", "a + circumflex -> â"},
40+
{"\u00E3", "a\u0303", "a + tilde -> ã"},
41+
{"\u00E4", "a\u0308", "a + diaeresis -> ä"},
42+
{"\u00E5", "a\u030A", "a + ring -> å"},
43+
{"\u0105", "a\u0328", "a + ogonek -> ą"},
44+
45+
{"\u00E9", "e\u0301", "e + acute -> é"}, // é
46+
{"\u00E8", "e\u0300", "e + grave -> è"}, // è
47+
{"\u00EA", "e\u0302", "e + circumflex -> ê"},
48+
{"\u00EB", "e\u0308", "e + diaeresis -> ë"},
49+
{"\u0119", "e\u0328", "e + ogonek -> ę"},
50+
{"\u011B", "e\u030C", "e + caron -> ě"},
51+
52+
{"\u00ED", "i\u0301", "i + acute -> í"},
53+
{"\u00EC", "i\u0300", "i + grave -> ì"},
54+
{"\u00EE", "i\u0302", "i + circumflex -> î"},
55+
{"\u00EF", "i\u0308", "i + diaeresis -> ï"},
56+
{"\u012B", "i\u0304", "i + macron -> ī"},
57+
58+
{"\u00F3", "o\u0301", "o + acute -> ó"},
59+
{"\u00F2", "o\u0300", "o + grave -> ò"},
60+
{"\u00F4", "o\u0302", "o + circumflex -> ô"},
61+
{"\u00F5", "o\u0303", "o + tilde -> õ"},
62+
{"\u00F6", "o\u0308", "o + diaeresis -> ö"},
63+
{"\u00F8", "o\u0338", "o + stroke (approx) -> ø"},
64+
65+
{"\u00F1", "n\u0303", "n + tilde -> ñ"},
66+
{"\u0144", "n\u0301", "n + acute -> ń"},
67+
68+
{"\u00E7", "c\u0327", "c + cedilla -> ç"},
69+
{"\u0107", "c\u0301", "c + acute -> ć"},
70+
{"\u010D", "c\u030C", "c + caron -> č"},
71+
72+
{"\u015B", "s\u0301", "s + acute -> ś"},
73+
{"\u0161", "s\u030C", "s + caron -> š"},
74+
75+
{"\u017A", "z\u0301", "z + acute -> ź"},
76+
{"\u017C", "z\u0307", "z + dot -> ż"},
77+
{"\u017E", "z\u030C", "z + caron -> ž"},
78+
79+
{"\u0142", "l\u0335", "l + stroke (approx) -> ł"},
80+
{"\u0159", "r\u030C", "r + caron -> ř"},
81+
{"\u0165", "t\u030C", "t + caron -> ť"},
82+
83+
{"\u0117", "e\u0307", "e + dot -> ė"},
84+
{"\u012F", "i\u0328", "i + ogonek -> į"},
85+
{"\u0173", "u\u0328", "u + ogonek -> ų"},
86+
87+
{"\u00FC", "u\u0308", "u + diaeresis -> ü"},
88+
{"\u016B", "u\u0304", "u + macron -> ū"},
89+
};
90+
91+
// Create labels for each pair and verify the stored text matches what we set
92+
for (size_t i = 0; i < std::size(pairs); ++i)
93+
{
94+
std::string name_pre = std::string("pre_") + std::to_string(i);
95+
std::string name_de = std::string("de_") + std::to_string(i);
96+
97+
LvLabel lp(name_pre, screen);
98+
LvLabel ld(name_de, screen);
99+
100+
lp.setText(pairs[i].pre);
101+
ld.setText(pairs[i].decomp);
102+
103+
ASSERT_STREQ(lp.getText(), pairs[i].pre) << "precomposed failed for " << pairs[i].desc;
104+
ASSERT_STREQ(ld.getText(), pairs[i].decomp) << "decomposed failed for " << pairs[i].desc;
105+
106+
// Precomposed and decomposed byte lengths will commonly differ
107+
EXPECT_NE(std::strlen(lp.getText()), std::strlen(ld.getText())) << pairs[i].desc;
108+
}
109+
110+
// Compose all pairs into single strings for a combined display on the two top-level labels
111+
std::string combined_pre;
112+
std::string combined_decomp;
113+
for (size_t i = 0; i < std::size(pairs); ++i)
114+
{
115+
if (!combined_pre.empty())
116+
{
117+
combined_pre += " ";
118+
combined_decomp += " ";
119+
}
120+
combined_pre += pairs[i].pre;
121+
combined_decomp += pairs[i].decomp;
122+
}
123+
124+
// Use the labels created at the top of the test to show the combined strings
125+
label_precomposed.setText(combined_pre);
126+
label_decomposed.setText(combined_decomp);
127+
128+
EXPECT_EQUAL_SCREENSHOT("diacritic/combined_vs_combining.png");
129+
}

0 commit comments

Comments
 (0)