Skip to content

Commit fc46a5e

Browse files
committed
Code style and minor fixes across libs
Apply consistent code style (remove redundant `const` on value params, remove duplicate `private:`, brace style, spacing) across display, displayudf, dmxnode, and pixeldmx libraries. Also fix printf format warnings with explicit casts, update network includes to use more specific headers, add `kTitleSize` constant, fix `vsnprintf` buffer size off-by-one, and update copyright year.
1 parent a7d687c commit fc46a5e

9 files changed

Lines changed: 138 additions & 184 deletions

File tree

lib-display/include/spi/display.h

Lines changed: 24 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -105,17 +105,17 @@ class Display : public LcdDriver {
105105
#else
106106
printf("ST7789 ");
107107
#endif
108-
printf("(%u,%u)\n", rows_, cols_);
108+
printf("(%u,%u)\n", static_cast<unsigned>(rows_), static_cast<unsigned>(cols_));
109109
}
110110

111111
void Cls() { FillColour(kColorBackground); }
112112

113-
void SetCursorPos(const uint32_t nCol, const uint32_t row) {
113+
void SetCursorPos(uint32_t nCol, uint32_t row) {
114114
cursor_x_ = nCol * s_pFONT->kWidth;
115115
cursor_y_ = row * s_pFONT->kHeight;
116116
}
117117

118-
void PutChar(const int c) {
118+
void PutChar(int c) {
119119
DrawChar(cursor_x_, cursor_y_, static_cast<char>(c), s_pFONT, kColorBackground, kColorForeground);
120120

121121
cursor_x_ += s_pFONT->kWidth;
@@ -137,7 +137,7 @@ class Display : public LcdDriver {
137137
}
138138
}
139139

140-
void ClearLine(const uint32_t nLine) {
140+
void ClearLine(uint32_t nLine) {
141141
if (__builtin_expect((!(nLine <= rows_)), 0)) {
142142
return;
143143
}
@@ -151,7 +151,7 @@ class Display : public LcdDriver {
151151
SetCursorPos(0, (nLine - 1U));
152152
}
153153

154-
void TextLine(const uint32_t nLine, const char* pText, const uint32_t nLength) {
154+
void TextLine(uint32_t nLine, const char* pText, const uint32_t nLength) {
155155
if (__builtin_expect((!(nLine <= rows_)), 0)) {
156156
return;
157157
}
@@ -162,18 +162,18 @@ class Display : public LcdDriver {
162162

163163
void ClearEndOfLine() { clear_end_of_line_ = true; }
164164

165-
void Text(const char* pData, uint32_t nLength) {
166-
if (nLength > cols_) {
167-
nLength = cols_;
165+
void Text(const char* data, uint32_t length) {
166+
if (length > cols_) {
167+
length = cols_;
168168
}
169169

170-
for (uint32_t i = 0; i < nLength; i++) {
171-
PutChar(pData[i]);
170+
for (uint32_t i = 0; i < length; i++) {
171+
PutChar(data[i]);
172172
}
173173
}
174174

175-
int Write(const uint32_t nLine, const char* pText) {
176-
const auto* p = pText;
175+
int Write(uint32_t line, const char* text) {
176+
const auto* p = text;
177177
int nCount = 0;
178178

179179
const auto columns = static_cast<int>(cols_);
@@ -182,28 +182,28 @@ class Display : public LcdDriver {
182182
++p;
183183
}
184184

185-
TextLine(nLine, pText, static_cast<uint8_t>(nCount));
185+
TextLine(line, text, static_cast<uint8_t>(nCount));
186186

187187
return nCount;
188188
}
189189

190-
int Printf(const uint8_t nLine, const char* format, ...) {
190+
int Printf(uint8_t line, const char* format, ...) {
191191
char buffer[32];
192192

193193
va_list arp;
194194

195195
va_start(arp, format);
196196

197-
auto i = vsnprintf(buffer, sizeof(buffer) / sizeof(buffer[0]), format, arp);
197+
const auto kSize = vsnprintf(buffer, sizeof(buffer) / sizeof(buffer[0]), format, arp);
198198

199199
va_end(arp);
200200

201-
TextLine(nLine, buffer, static_cast<uint16_t>(i));
201+
TextLine(line, buffer, static_cast<uint16_t>(kSize));
202202

203-
return i;
203+
return kSize;
204204
}
205205

206-
void TextStatus(const char* pText) {
206+
void TextStatus(const char* text) {
207207
SetCursorPos(0, static_cast<uint8_t>(rows_ - 1));
208208

209209
for (uint32_t i = 0; i < (cols_ - 1); i++) {
@@ -212,7 +212,7 @@ class Display : public LcdDriver {
212212

213213
SetCursorPos(0, static_cast<uint8_t>(rows_ - 1));
214214

215-
Write(rows_, pText);
215+
Write(rows_, text);
216216
}
217217

218218
void TextStatus(const char* text, ansi::Colours::Colour colour) {
@@ -221,20 +221,20 @@ class Display : public LcdDriver {
221221
}
222222

223223
void Progress() {
224-
static constexpr char SYMBOLS[] = {'/', '-', '\\', '|'};
224+
static constexpr char kSymbols[] = {'/', '-', '\\', '|'};
225225
static uint32_t nSymbolsIndex;
226226

227227
SetCursorPos(GetColumns() - 1U, GetRows() - 1U);
228-
PutChar(SYMBOLS[nSymbolsIndex++]);
228+
PutChar(kSymbols[nSymbolsIndex++]);
229229

230-
if (nSymbolsIndex >= sizeof(SYMBOLS)) {
230+
if (nSymbolsIndex >= sizeof(kSymbols)) {
231231
nSymbolsIndex = 0;
232232
}
233233
}
234234

235-
void SetContrast(const uint8_t nContrast) { SetBackLight(nContrast); }
235+
void SetContrast(uint8_t nContrast) { SetBackLight(nContrast); }
236236

237-
void SetSleep(const bool bSleep) {
237+
void SetSleep(bool bSleep) {
238238
is_sleep_ = bSleep;
239239

240240
EnableSleep(bSleep);
@@ -282,7 +282,6 @@ class Display : public LcdDriver {
282282
private:
283283
void SetSleepTimer(const bool bActive);
284284

285-
private:
286285
uint32_t cols_;
287286
uint32_t rows_;
288287
uint32_t sleep_timeout_{1000U * 60U * display::Defaults::kSleepTimeout};

lib-displayudf/include/displayudf.h

Lines changed: 25 additions & 48 deletions
Original file line numberDiff line numberDiff line change
@@ -27,12 +27,13 @@
2727
#define DISPLAYUDF_H_
2828

2929
#include <cstdint>
30-
#include <cstdarg>
3130

32-
#include "display.h"
33-
#include "firmware/firmwareversion.h"
31+
#include "display.h" // IWYU pragma: keep
32+
#include "firmware/firmwareversion.h" // IWYU pragma: keep
3433
#if !defined(NO_EMAC)
35-
#include "network.h"
34+
#include "network_config.h"
35+
#include "network_iface.h"
36+
#include "ip4/ip4_address.h"
3637
#include "core/protocol/dhcp.h"
3738
#endif
3839
#if defined(NODE_ARTNET_MULTI)
@@ -57,14 +58,12 @@
5758
#if defined(RDM_RESPONDER) || defined(OUTPUT_DMX_MONITOR) || defined(OUTPUT_DMX_PCA9685) || defined(OUTPUT_DMX_PIXEL) || defined(OUTPUT_DMX_TLC59711)
5859
#define HAVE_DMX_START_ADDRESS
5960
#endif
60-
#include "firmware/debug/debug_debug.h"
6161

62-
namespace displayudf
63-
{
62+
namespace displayudf {
6463
inline constexpr uint32_t kLabelMaxRows = 6;
64+
inline constexpr uint32_t kTitleSize = 32;
6565

66-
enum class Labels : uint8_t
67-
{
66+
enum class Labels : uint8_t {
6867
kTitle,
6968
kBoardname,
7069
kVersion,
@@ -101,14 +100,12 @@ enum class Labels : uint8_t
101100
kUnknown
102101
};
103102

104-
namespace defaults
105-
{
103+
namespace defaults {
106104
inline constexpr uint8_t kIntensity = 0x7F;
107105
} // namespace defaults
108106
} // namespace displayudf
109107

110-
class DisplayUdf final : public Display
111-
{
108+
class DisplayUdf final : public Display {
112109
public:
113110
DisplayUdf();
114111

@@ -120,10 +117,8 @@ class DisplayUdf final : public Display
120117
void SetTitle(const char* format, ...);
121118
void Set(uint32_t line, displayudf::Labels label);
122119

123-
uint8_t GetLabel(uint32_t index) const
124-
{
125-
if (index < static_cast<uint32_t>(displayudf::Labels::kUnknown))
126-
{
120+
uint8_t GetLabel(uint32_t index) const {
121+
if (index < static_cast<uint32_t>(displayudf::Labels::kUnknown)) {
127122
return labels_[index];
128123
}
129124

@@ -144,8 +139,7 @@ class DisplayUdf final : public Display
144139
*/
145140

146141
#if defined(RDM_RESPONDER)
147-
void ShowDmxStartAddress()
148-
{
142+
void ShowDmxStartAddress() {
149143
const auto dmx_start_address = RDMDeviceResponder::Get()->GetDmxStartAddress();
150144
const auto nDmxFootprint = RDMDeviceResponder::Get()->GetDmxFootPrint();
151145
Printf(labels_[static_cast<uint32_t>(displayudf::Labels::kDmxStartAddress)], "DMX S:%3u F:%3u", dmx_start_address, nDmxFootprint);
@@ -157,53 +151,44 @@ class DisplayUdf final : public Display
157151
*/
158152

159153
#if !defined(NO_EMAC)
160-
void ShowEmacInit()
161-
{
154+
void ShowEmacInit() {
162155
ClearEndOfLine();
163156
Printf(labels_[static_cast<uint32_t>(displayudf::Labels::kIp)], "Ethernet init");
164157
}
165158

166-
void ShowEmacStart()
167-
{
159+
void ShowEmacStart() {
168160
ClearEndOfLine();
169161
Printf(labels_[static_cast<uint32_t>(displayudf::Labels::kIp)], "Ethernet start");
170162
}
171163

172-
void ShowEmacStatus(bool is_link_up)
173-
{
164+
void ShowEmacStatus(bool is_link_up) {
174165
ClearEndOfLine();
175166
Printf(labels_[static_cast<uint32_t>(displayudf::Labels::kIp)], "Ethernet Link %s", is_link_up ? "UP" : "DOWN");
176167
}
177168

178-
void ShowIpAddress()
179-
{
169+
void ShowIpAddress() {
180170
ClearEndOfLine();
181171
Printf(labels_[static_cast<uint32_t>(displayudf::Labels::kIp)], "" IPSTR "/%d %c", IP2STR(network::GetPrimaryIp()), network::GetNetmaskCIDR(), network::iface::AddressingMode());
182172
}
183173

184-
void ShowNetmask()
185-
{
174+
void ShowNetmask() {
186175
ClearEndOfLine();
187176
Printf(labels_[static_cast<uint32_t>(displayudf::Labels::kNetmask)], "N: " IPSTR "", IP2STR(network::GetNetmask()));
188177
ShowIpAddress();
189178
}
190179

191-
void ShowGatewayIp()
192-
{
180+
void ShowGatewayIp() {
193181
ClearEndOfLine();
194182
Printf(labels_[static_cast<uint32_t>(displayudf::Labels::kDefaultGateway)], "G: " IPSTR "", IP2STR(network::GetGatewayIp()));
195183
}
196184

197-
void ShowHostName()
198-
{
185+
void ShowHostName() {
199186
ClearEndOfLine();
200187
Write(labels_[static_cast<uint32_t>(displayudf::Labels::kHostname)], network::iface::HostName());
201188
}
202189

203-
void ShowDhcpStatus(network::dhcp::State state)
204-
{
205-
switch (state)
206-
{
190+
void ShowDhcpStatus(network::dhcp::State state) {
191+
switch (state) {
207192
case network::dhcp::State::kOff:
208193
break;
209194
case network::dhcp::State::kRenewing:
@@ -223,25 +208,17 @@ class DisplayUdf final : public Display
223208
static DisplayUdf* Get() { return s_this; }
224209

225210
private:
226-
/**
227-
* Art-Net
228-
*/
229-
211+
// Art-Net
230212
#if defined(NODE_ARTNET)
231213
void ShowArtNetNode();
232214
void ShowDestinationIpArtNetNode();
233215
#endif
234-
235-
/**
236-
* sACN E1.31
237-
*/
238-
216+
// sACN E1.31
239217
#if defined(NODE_E131)
240218
void ShowE131Bridge();
241219
#endif
242220

243-
private:
244-
char title_[32];
221+
char title_[displayudf::kTitleSize];
245222
uint8_t labels_[static_cast<uint32_t>(displayudf::Labels::kUnknown)];
246223

247224
inline static DisplayUdf* s_this;

lib-displayudf/include/json/displayudfparams.h

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -35,11 +35,9 @@
3535

3636
static_assert(common::ArraySize(json::DisplayUdfParamsConst::kLabels) == static_cast<size_t>(displayudf::Labels::kUnknown), "Mismatch between enum and kArray");
3737

38-
namespace json
39-
{
40-
class DisplayUdfParams : public JsonParamsBase<DisplayUdfParams>
41-
{
42-
public:
38+
namespace json {
39+
class DisplayUdfParams : public JsonParamsBase<DisplayUdfParams> {
40+
public:
4341

4442
DisplayUdfParams();
4543

lib-displayudf/src/displayudf.cpp

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -47,17 +47,17 @@ void DisplayUdf::SetTitle(const char* format, ...) {
4747
va_list arp;
4848
va_start(arp, format);
4949

50-
const auto kI = vsnprintf(title_, sizeof(title_) / sizeof(title_[0]) - 1, format, arp);
50+
const auto kIndex = vsnprintf(title_, sizeof(title_) / sizeof(title_[0]), format, arp);
5151

5252
va_end(arp);
5353

54-
title_[kI] = '\0';
54+
title_[kIndex] = '\0';
5555

5656
DEBUG_PUTS(title_);
5757
}
5858

5959
void DisplayUdf::Set(uint32_t line, displayudf::Labels label) {
60-
if (!((line > 0) && (line <= displayudf::kLabelMaxRows))) {
60+
if ((line == 0) || (line > displayudf::kLabelMaxRows)) {
6161
return;
6262
}
6363

lib-displayudf/src/json/displayudfparams.cpp

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -74,7 +74,7 @@ void DisplayUdfParams::SetLabel(const char* key, uint32_t key_len, const char* v
7474
return;
7575
}
7676

77-
DEBUG_PRINTF("%.*s ->%.*s", key_len, key, val_len, val);
77+
DEBUG_PRINTF("%.*s ->%.*s", static_cast<int>(key_len), key, static_cast<int>(val_len), val);
7878

7979
const uint32_t kHash = Fnv1a32Runtime(key, key_len);
8080
bool matched = false;
@@ -132,12 +132,12 @@ void DisplayUdfParams::SetAndShow() {
132132

133133
void DisplayUdfParams::Dump() {
134134
printf("%s::%s \'%s\':\n", __FILE__, __FUNCTION__, json::DisplayUdfParamsConst::kFileName);
135-
printf(" %s=%u\n", DisplayUdfParamsConst::kIntensity.name, store_displayudf.intensity);
136-
printf(" %s=%u\n", DisplayUdfParamsConst::kSleepTimeout.name, store_displayudf.sleep_timeout);
137-
printf(" %s=%u\n", DisplayUdfParamsConst::kFlipVertically.name, common::IsFlagSet(store_displayudf.flags, Flags::Flag::kFlipVertically));
135+
printf(" %s=%u\n", DisplayUdfParamsConst::kIntensity.name, static_cast<unsigned>(store_displayudf.intensity));
136+
printf(" %s=%u\n", DisplayUdfParamsConst::kSleepTimeout.name, static_cast<unsigned>(store_displayudf.sleep_timeout));
137+
printf(" %s=%u\n", DisplayUdfParamsConst::kFlipVertically.name, static_cast<unsigned>(common::IsFlagSet(store_displayudf.flags, Flags::Flag::kFlipVertically)));
138138

139139
for (uint32_t i = 0; i < common::ArraySize(DisplayUdfParamsConst::kLabels); ++i) {
140-
printf(" %s=%u\n", DisplayUdfParamsConst::kLabels[i].name, store_displayudf.label_index[i]);
140+
printf(" %s=%u\n", DisplayUdfParamsConst::kLabels[i].name, static_cast<unsigned>(store_displayudf.label_index[i]));
141141
}
142142
}
143143
} // namespace json

0 commit comments

Comments
 (0)