-
-
Notifications
You must be signed in to change notification settings - Fork 228
Suggested Improvements
This document provides actionable recommendations for improving the FPP codebase architecture, organization, and maintainability. Recommendations are prioritized by impact and effort.
Current Issue: fppd.cpp is 1167 lines with mixed concerns.
Recommendation: Split into focused modules.
src/
daemon/
fppd.cpp # Main entry point only
DaemonManager.cpp # Daemon lifecycle (fork, setsid, restart)
SignalHandler.cpp # Signal handling and crash recovery
MainLoop.cpp # Event loop implementation
Benefits:
- Each file has a single responsibility
- Easier to navigate and understand
- Better testability
Migration Path:
- Extract signal handling to
SignalHandler.cpp - Extract daemon lifecycle to
DaemonManager.cpp - Extract event loop to
MainLoop.cpp - Update
fppd.cppto orchestrate
Current Issue: Files scattered across src/ without clear domain boundaries.
Recommendation: Reorganize into domain directories.
src/
daemon/ # fppd main process
sequence/ # FSEQ playback
player/ # Playlist state machine
bridge/ # E1.31/DDP/ArtNet
plugins/ # Plugin system
channeloutput/ # Channel outputs (keep existing)
overlays/ # Pixel overlays (keep existing)
config/ # Configuration management
util/ # Utilities (keep existing)
Migration Path:
- Create new directory structure
- Move files incrementally
- Update includes one file at a time
- Test after each move
Current Issue: Multiple global singletons (sequence, multiSync, scheduler, PluginManager::INSTANCE).
Recommendation: Use dependency injection.
// Before
extern Sequence* sequence;
extern MultiSync* multiSync;
extern Scheduler* scheduler;
// After
class Daemon {
private:
std::unique_ptr<Sequence> sequence;
std::unique_ptr<MultiSync> multiSync;
std::unique_ptr<Scheduler> scheduler;
std::unique_ptr<Player> player;
public:
void run();
};Benefits:
- Easier to test
- Clearer dependencies
- Better lifecycle management
Current Issue: Main loop blocks on EPoll while processing.
Recommendation: Decouple event handling from business logic.
class EventLoop {
public:
void run(std::function<void()> onEvent);
};
class ProcessingLoop {
public:
void run();
};
// Main
EventLoop eventLoop;
ProcessingLoop processingLoop;
eventLoop.run([&]() {
processingLoop.process();
});Benefits:
- Better responsiveness
- Easier to add parallelism
- Clearer separation of concerns
Current Issue: Bridge data merge happens in main processing loop.
Recommendation: Separate bridge processing thread.
class BridgeThread {
public:
void start();
void stop();
void pushBridgeData();
private:
std::thread thread;
std::queue<Packet> packetQueue;
};Benefits:
- Non-blocking bridge handling
- Better latency isolation
- Easier to debug
Current Issue: Inconsistent thread management (some detached, some not).
Recommendation: Unified thread lifecycle management.
class ThreadManager {
public:
template<typename F>
void start(const char* name, F&& func) {
threads.emplace_back(std::thread(std::forward<F>(func)));
}
void waitForAll() {
for (auto& t : threads) {
if (t.joinable()) t.join();
}
}
private:
std::vector<std::thread> threads;
};Benefits:
- Consistent lifecycle
- Easier cleanup
- Better error handling
Current Issue: Raw pointers throughout codebase.
// Before
Sequence* sequence = new Sequence();
// ... use ...
delete sequence;
// After
std::unique_ptr<Sequence> sequence = std::make_unique<Sequence>();
// ... use ...
// Automatic cleanupBenefits:
- No memory leaks
- Clear ownership
- Exception safety
Current Issue: Manual shared memory management.
// Before
int shm_fd = shm_open("/FPP-Model-Data-{name}", O_RDWR, 0666);
void* ptr = mmap(NULL, size, PROT_READ | PROT_WRITE, MAP_SHARED, shm_fd, 0);
// ... use ...
munmap(ptr, size);
shm_unlink("/FPP-Model-Data-{name}");
// After
class SharedMemory {
public:
SharedMemory(const char* name, size_t size) {
fd = shm_open(name, O_RDWR, 0669);
ptr = mmap(...);
}
~SharedMemory() {
munmap(ptr, size);
shm_unlink(name);
}
void* get() { return ptr; }
private:
int fd;
void* ptr;
size_t size;
};Benefits:
- No resource leaks
- Exception safety
- Simpler code
Current Issue: Manual dlopen/dlclose with raw handles.
// Before
void* handle = dlopen("libfpp-co-E131.so", RTLD_LAZY);
ChannelOutput* output = createChannelOutput();
// ... use ...
dlclose(handle);
// After
class PluginHandle {
public:
PluginHandle(const char* path) : handle(dlopen(path, RTLD_LAZY)) {}
~PluginHandle() { if (handle) dlclose(handle); }
void* symbol(const char* name) { return dlsym(handle, name); }
private:
void* handle;
};Benefits:
- Automatic cleanup
- No leaks
- Exception safety
Current Issue: Error handling via logging and return codes.
// Before
if (!openFile()) {
LogErr("Failed to open file");
return -1;
}
// After
enum class ErrorCode {
FILE_NOT_FOUND,
PERMISSION_DENIED,
INSUFFICIENT_RESOURCES
};
struct Error {
ErrorCode code;
std::string message;
};
std::optional<Error> openFile(const std::string& path) {
FILE* f = fopen(path.c_str(), "r");
if (!f) {
return Error{ErrorCode::FILE_NOT_FOUND, "Cannot open " + path};
}
return std::nullopt;
}Benefits:
- Programmatic error handling
- Better error messages
- Easier testing
Current Issue: Silent failures in some areas.
// Before
auto result = readFile();
if (result.empty()) {
LogErr("File empty");
}
processData(); // May fail silently
// After
auto result = readFile();
if (auto err = result.error()) {
return err; // Propagate
}
auto data = result.value();
auto processed = processData(data);
if (auto err = processed.error()) {
return err; // Propagate
}Benefits:
- Errors not ignored
- Better debugging
- Easier testing
Current Issue: Limited error recovery.
// Before
if (!connect()) {
exit(1);
}
// After
if (!connect()) {
LogWarn("Connection failed, retrying...");
if (retry(3, []{ return connect(); })) {
LogInfo("Reconnected");
} else {
LogErr("Failed to reconnect");
return Error{ErrorCode::CONNECTION_FAILED};
}
}Benefits:
- Better resilience
- User-friendly
- Easier debugging
Current Issue: No visible unit tests.
// src/channeloutput/processors/test/
// RemapOutputProcessorTest.cpp
TEST(RemapOutputProcessor, MapsChannels) {
RemapOutputProcessor processor;
processor.setOutputMap({0, 1, 2, 3});
EXPECT_EQ(processor.process(0), 0);
EXPECT_EQ(processor.process(1), 1);
}
// ThreeToFourOutputProcessorTest.cpp
TEST(ThreeToFourOutputProcessor, ExpandsRGB) {
ThreeToFourOutputProcessor processor;
EXPECT_EQ(processor.process(255), 0);
EXPECT_EQ(processor.process(128), 0);
EXPECT_EQ(processor.process(64), 255);
}Benefits:
- Regression prevention
- Documentation
- Confidence in changes
Current Issue: Hard to test without hardware.
class MockChannelOutput : public ChannelOutput {
public:
std::vector<uint8_t> lastData;
int lastChannelCount = 0;
void Init(int startChannel, int channelCount) override {
lastChannelCount = channelCount;
}
void SendData(const uint8_t* data) override {
lastData.assign(data, data + lastChannelCount * 3);
}
};Benefits:
- Test without hardware
- Faster tests
- Easier debugging
Current Issue: Manual testing via CLI.
// tests/integration/
// ChannelOutputTest.cpp
TEST_F(ChannelOutputIntegration, E131Output) {
MockChannelOutput output;
output.Init(0, 512);
uint8_t data[512 * 3];
memset(data, 0, sizeof(data));
data[0] = 255;
output.SendData(data);
EXPECT_EQ(output.lastChannelCount, 512);
EXPECT_EQ(output.lastData[0], 255);
}Benefits:
- Automated testing
- CI integration
- Regression prevention
Current Issue: Minimal inline documentation.
// Before
class ChannelOutput {
public:
virtual void Init(int startChannel, int channelCount) = 0;
virtual void SendData(const uint8_t* data) = 0;
};
// After
/// Channel output interface for LED control.
///
/// Implement this class to create a new channel output plugin.
class ChannelOutput {
public:
/// Initialize the channel output.
/// @param startChannel First channel number
/// @param channelCount Number of channels
virtual void Init(int startChannel, int channelCount) = 0;
/// Send channel data to the output.
/// @param data Channel data (3 bytes per channel: RGB)
virtual void SendData(const uint8_t* data) = 0;
};Benefits:
- Auto-generated API docs
- Better IDE support
- Easier onboarding
Current Issue: Architecture not well documented.
docs/adr/
0001-record-architecture-decisions.md
0002-use-epoll-for-event-loop.md
0003-single-buffer-data-pipeline.md
0004-plugin-based-extensibility.md
Template:
# ADR 0001: Record Architecture Decisions
## Status
Accepted
## Context
What is the issue we're trying to solve?
## Decision
What did we decide to do?
## Consequences
What are the resulting consequences?Benefits:
- Design rationale
- Future reference
- Onboarding
Current Issue: Limited plugin documentation.
docs/plugins/
getting-started.md
channel-output-plugins.md
playlist-event-plugins.md
channel-data-plugins.md
api-provider-plugins.md
examples/
hello-world.md
custom-output.md
Benefits:
- Easier plugin development
- Community growth
- Consistency
Current Issue: JSON loaded without validation.
// Before
Json::Value config;
file >> config; // May fail at runtime
// After
struct ChannelOutputConfig {
std::string type;
int startChannel;
int channelCount;
};
std::optional<Error> validate(const ChannelOutputConfig& config) {
if (config.startChannel < 0) {
return Error{"startChannel must be >= 0"};
}
if (config.channelCount <= 0) {
return Error{"channelCount must be > 0"};
}
return std::nullopt;
}Benefits:
- Earlier error detection
- Better error messages
- User-friendly
Current Issue: No config validation tool.
# fpp config validate
# Validates all config files and reports errors
$ fpp config validate
Validating /etc/fpp/channeloutputs.json... OK
Validating /etc/fpp/model-overlays.json... OK
Validating /media/settings... OK
All configurations valid.Benefits:
- Pre-deployment validation
- Better UX
- Fewer runtime errors
Current Issue: Config changes require restart.
class ConfigManager {
public:
void startHotReloading(); // Watch for changes
};Benefits:
- No restart needed
- Better UX
- Faster iteration
Current Issue: No versioning mechanism.
// Before
class ChannelOutputPlugin {
public:
virtual ChannelOutput* createChannelOutput(...) = 0;
};
// After
enum class PluginVersion { V1 = 1 };
class ChannelOutputPlugin {
public:
virtual PluginVersion getVersion() = 0;
virtual ChannelOutput* createChannelOutput(...) = 0;
};Benefits:
- Backward compatibility
- Breaking change detection
- Migration path
Current Issue: Plugins don't declare capabilities.
class ChannelOutputPlugin {
public:
struct Capabilities {
bool supportsUnicast = true;
bool supportsMulticast = true;
int maxChannels = 2097152;
};
virtual Capabilities getCapabilities() = 0;
};Benefits:
- Better error messages
- Feature detection
- Configuration help
Current Issue: Plugins run with full privileges.
// Future: Run plugins in sandboxed environment
// - Seccomp filters
// - Namespace isolation
// - Resource limitsBenefits:
- Security
- Stability
- Multi-tenant support
Current Issue: Bridge data uses mutex.
// Before
std::mutex m_bridgeRangesLock;
// After
std::atomic<uint64_t> m_bridgeDataExpiry[FPPD_MAX_CHANNELS];Benefits:
- No locking overhead
- Better scalability
- Lower latency
Current Issue: Frame allocation on heap.
// Before
FSEQFile::FrameData* frame = new FSEQFile::FrameData();
// After
class FramePool {
public:
FSEQFile::FrameData* acquire() {
if (pool.empty()) return new FSEQFile::FrameData();
auto frame = pool.back();
pool.pop_back();
return frame;
}
void release(FSEQFile::FrameData* frame) {
pool.push_back(frame);
}
private:
std::vector<FSEQFile::FrameData*> pool;
};Benefits:
- No allocation overhead
- Better cache locality
- Predictable latency
Current Issues:
- PHP 7.x with legacy code patterns
- Limonade micro-framework (2009) - unmaintained
- Mixed concerns in
www/common.php - No type safety
- Global state (
$settings,$pluginSettings)
Recommendations:
// Before
function ReadSettingFromFile($settingName, $plugin = "") {
global $settingsFile;
global $settings;
$filename = $settingsFile;
// ... file reading ...
}
// After
class SettingsRepository {
public function __construct(private string $settingsFile) {}
public function get(string $name): ?string {
$settings = $this->load();
return $settings[$name] ?? null;
}
public function set(string $name, string $value): void {
$settings = $this->load();
$settings[$name] = $value;
$this->save($settings);
}
private function load(): array {
$lines = file($this->settingsFile, FILE_IGNORE_NEW_LINES | FILE_SKIP_EMPTY_LINES);
return array_map(fn($line) => explode('=', $line, 2), $lines);
}
}Benefits:
- Type safety
- Testability
- Modern PHP (8.x)
- Better IDE support
Current Issues:
- Controllers embedded in
www/api/controllers/ - No clear API versioning
- Inconsistent response formats
- No authentication for some endpoints
Recommendations:
www/
api/
v1/
controllers/
middleware/
responses/
v2/
controllers/
Benefits:
- Versioned APIs
- Cleaner separation
- Better security
Current Issues:
- jQuery + Bootstrap 3 (2016)
- Server-side rendering
- No component architecture
Recommendations:
- Progressive enhancement to SPA
- Vue.js or React for complex UIs
- RESTful API backend
- WebSockets for real-time updates
Benefits:
- Better UX
- Faster interactions
- Mobile-friendly
Current Issues:
- Limited input validation
- Some endpoints unauthenticated
- File operations with user input
Recommendations:
- CSRF protection
- Input validation
- Rate limiting
- Audit logging
Current Issue: Platform detection scattered.
// Before
#ifdef PLATFORM_PI
// Pi code
#elif defined(PLATFORM_BBB)
// BBB code
#endif
// After
class Platform {
public:
enum class Type { RaspberryPi, BeagleBone, Linux, macOS };
static Type getType();
static bool hasGPIO();
static bool hasSPI();
};
if (Platform::getType() == Platform::Type::RaspberryPi) {
// Pi code
}Benefits:
- Cleaner code
- Easier testing
- Better portability
- Add Doxygen comments to public APIs
- Create architecture documentation
- Add config validation CLI
- Write plugin developer guide
- Extract signal handling to
SignalHandler.cpp - Reorganize files by domain
- Add unit tests for core algorithms
- Create mock channel outputs
- Separate event loop from processing
- Add dedicated bridge thread
- Standardize thread lifecycle
- Add integration tests
- Replace raw pointers with smart pointers
- RAII for shared memory
- Plugin memory management
- Memory leak detection
- Structured error types
- Error propagation
- Recovery mechanisms
- Error documentation
- Versioned plugin interfaces
- Capability negotiation
- Plugin documentation
- Example plugins
| Priority | Area | Impact | Effort | Timeline |
|---|---|---|---|---|
| 1 | Code Organization | High | Medium | Weeks 1-2 |
| 2 | Threading Model | High | High | Weeks 5-8 |
| 3 | Memory Management | Medium | Medium | Weeks 9-10 |
| 4 | Error Handling | Medium | Medium | Weeks 11-12 |
| 5 | Testing Infrastructure | High | Medium | Weeks 3-4 |
| 6 | Documentation | Medium | Low | Weeks 1-2 |
| 7 | Configuration System | Medium | Low | Weeks 1-2 |
| 8 | Plugin System | Medium | Medium | Weeks 13-14 |
| 9 | Web UI Layer | Medium | Medium | Weeks 15-16 |
| 10 | Platform Support | Low | Medium | Weeks 1-2 |
Generated: 2026-04-01