Skip to content

Commit

Permalink
Alway supply an empty Playlist
Browse files Browse the repository at this point in the history
Make sure there is always a dummy playlist and move initialization of gPlayProperties into the AudioPlayer Task.
Fix formatting issues and the review points

- Add guards to prevent access to playlist if pointer is nullptr. This will be improved in PR 2 of 3 by encapsulating playlist and preventing direct access.
- remove commented code parts
- fix spelling
  • Loading branch information
laszloh committed Dec 14, 2023
1 parent c8d40a2 commit 6282d8e
Show file tree
Hide file tree
Showing 8 changed files with 22 additions and 15 deletions.
14 changes: 9 additions & 5 deletions src/AudioPlayer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -118,9 +118,14 @@ void AudioPlayer_Init(void) {
// Adjust volume depending on headphone is connected and volume-adjustment is enabled
AudioPlayer_SetupVolumeAndAmps();

// initialize gPlayProperties
memset(&gPlayProperties, 0, sizeof(gPlayProperties));
gPlayProperties.playlistFinished = true;

// clear title and cover image
gPlayProperties.title[0] = '\0';
gPlayProperties.coverFilePos = 0;
gPlayProperties.playlist = new Playlist();

// Don't start audio-task in BT-speaker mode!
if ((System_GetOperationMode() == OPMODE_NORMAL) || (System_GetOperationMode() == OPMODE_BLUETOOTH_SOURCE)) {
Expand Down Expand Up @@ -359,12 +364,12 @@ void AudioPlayer_Task(void *parameter) {
}

uint8_t currentVolume;
static BaseType_t trackQStatus;
static uint8_t trackCommand = NO_ACTION;
BaseType_t trackQStatus = pdFAIL;
uint8_t trackCommand = NO_ACTION;
bool audioReturnCode;
AudioPlayer_CurrentTime = 0;
AudioPlayer_FileDuration = 0;
static uint32_t AudioPlayer_LastPlaytimeStatsTimestamp = 0u;
uint32_t AudioPlayer_LastPlaytimeStatsTimestamp = 0u;

for (;;) {
/*
Expand Down Expand Up @@ -407,7 +412,7 @@ void AudioPlayer_Task(void *parameter) {
if (trackQStatus == pdPASS) {
audio->stopSong();

// destory the old playlist and assign the new
// destroy the old playlist and assign the new
freePlaylist(gPlayProperties.playlist);
gPlayProperties.playlist = newPlaylist;
Log_Printf(LOGLEVEL_NOTICE, newPlaylistReceived, gPlayProperties.playlist->size());
Expand Down Expand Up @@ -668,7 +673,6 @@ void AudioPlayer_Task(void *parameter) {
publishMqtt(topicPlaymodeState, gPlayProperties.playMode, false);
#endif
gPlayProperties.currentTrackNumber = 0;
// gPlayProperties.numberOfTracks = 0;
if (gPlayProperties.sleepAfterPlaylist) {
System_RequestSleep();
}
Expand Down
2 changes: 1 addition & 1 deletion src/Cmd.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -139,7 +139,7 @@ void Cmd_Action(const uint16_t mod) {
}

case CMD_SLEEP_AFTER_5_TRACKS: {
if (gPlayProperties.playMode == NO_PLAYLIST) {
if (gPlayProperties.playMode == NO_PLAYLIST || !gPlayProperties.playlist) {
Log_Println(modificatorNotallowedWhenIdle, LOGLEVEL_NOTICE);
System_IndicateError();
return;
Expand Down
5 changes: 3 additions & 2 deletions src/Led.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -940,8 +940,9 @@ AnimationReturnType Animation_PlaylistProgress(const bool startNewAnimation, CRG
static uint32_t staticLastTrack = 0; // variable to remember the last track (for connecting animations)

if constexpr (NUM_INDICATOR_LEDS >= 4) {
if (gPlayProperties.playlist->size() > 1 && gPlayProperties.currentTrackNumber < gPlayProperties.playlist->size()) {
const uint32_t ledValue = std::clamp<uint32_t>(map(gPlayProperties.currentTrackNumber, 0, gPlayProperties.playlist->size() - 1, 0, leds.size() * DIMMABLE_STATES), 0, leds.size() * DIMMABLE_STATES);
const uint16_t currentTrack = (gPlayProperties.playlist) ? gPlayProperties.playlist->size() : 0;
if (currentTrack > 1 && gPlayProperties.currentTrackNumber < currentTrack) {
const uint32_t ledValue = std::clamp<uint32_t>(map(gPlayProperties.currentTrackNumber, 0, currentTrack - 1, 0, leds.size() * DIMMABLE_STATES), 0, leds.size() * DIMMABLE_STATES);
const uint8_t fullLeds = ledValue / DIMMABLE_STATES;
const uint8_t lastLed = ledValue % DIMMABLE_STATES;

Expand Down
5 changes: 5 additions & 0 deletions src/Mqtt.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -335,6 +335,11 @@ void Mqtt_ClientCallback(const char *topic, const byte *payload, uint32_t length
free(receivedString);
return;
} else if (strcmp(receivedString, "EO5T") == 0) {
if (gPlayProperties.playMode == NO_PLAYLIST || !gPlayProperties.playlist) {
Log_Println(modificatorNotallowedWhenIdle, LOGLEVEL_NOTICE);
System_IndicateError();
return;
}
if ((gPlayProperties.playlist->size() - 1) >= (gPlayProperties.currentTrackNumber + 5)) {
gPlayProperties.playUntilTrackNumber = gPlayProperties.currentTrackNumber + 5;
} else {
Expand Down
2 changes: 1 addition & 1 deletion src/SdCard.h
Original file line number Diff line number Diff line change
Expand Up @@ -16,5 +16,5 @@ sdcard_type_t SdCard_GetType(void);
uint64_t SdCard_GetSize();
uint64_t SdCard_GetFreeSize();
void SdCard_PrintInfo();
std::optional<Playlist*> SdCard_ReturnPlaylist(const char *fileName, const uint32_t _playMode);
std::optional<Playlist *> SdCard_ReturnPlaylist(const char *fileName, const uint32_t _playMode);
const String SdCard_pickRandomSubdirectory(const char *_directory);
4 changes: 2 additions & 2 deletions src/Web.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1005,7 +1005,7 @@ void Web_SendWebsocketData(uint32_t client, uint8_t code) {
JsonObject entry = object.createNestedObject("trackinfo");
entry["pausePlay"] = gPlayProperties.pausePlay;
entry["currentTrackNumber"] = gPlayProperties.currentTrackNumber + 1;
entry["numberOfTracks"] = gPlayProperties.playlist->size();
entry["numberOfTracks"] = (gPlayProperties.playlist) ? gPlayProperties.playlist->size() : 0;
entry["volume"] = AudioPlayer_GetCurrentVolume();
entry["name"] = gPlayProperties.title;
entry["posPercent"] = gPlayProperties.currentRelPos;
Expand Down Expand Up @@ -1890,7 +1890,7 @@ void Web_DumpSdToNvs(const char *_filename) {
// handle album cover image request
static void handleCoverImageRequest(AsyncWebServerRequest *request) {

if (!gPlayProperties.coverFilePos) {
if (!gPlayProperties.coverFilePos || !gPlayProperties.playlist) {
// empty image:
// request->send(200, "image/svg+xml", "<?xml version=\"1.0\"?><svg xmlns=\"http://www.w3.org/2000/svg\"/>");
if (gPlayProperties.playMode == WEBSTREAM) {
Expand Down
3 changes: 0 additions & 3 deletions src/main.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -159,9 +159,6 @@ void setup() {
// All checks that could send us to sleep are done, power up fully
Power_PeripheralOn();

memset(&gPlayProperties, 0, sizeof(gPlayProperties));
gPlayProperties.playlistFinished = true;

Led_Init();

// Only used for ESP32-A1S-Audiokit
Expand Down
2 changes: 1 addition & 1 deletion src/settings.h
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
#define __ESPUINO_SETTINGS_H__
#include "Arduino.h"
#include "values.h"
#include "cpp.h"
#include "cpp.h"
#if __has_include("settings-override.h")
#include "settings-override.h"
#else
Expand Down

0 comments on commit 6282d8e

Please sign in to comment.