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.

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

Global sound settings implemented and sound menu added #143

Closed
wants to merge 0 commits into from

Conversation

noisedsn
Copy link
Contributor

sdk: global sound settings implemented.
keira: global sound settings implemented and sound menu added.
doom: global sound settings implemented.

@noisedsn noisedsn changed the title Global sound settings implemented and sound menu Global sound settings implemented and sound menu added Dec 23, 2024
Copy link
Collaborator

@black-ghost-off black-ghost-off left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Collaborator

@black-ghost-off black-ghost-off left a comment

Choose a reason for hiding this comment

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

CLang-format not passed, CPPCheck not passed, fix it

@noisedsn
Copy link
Contributor Author

Справедливо. Ще вчора я про cppcheck нічого не знав, а позавчора про clang-format. Наче пофіксав, в мене проходять обидва тести тепер.

@@ -242,7 +242,7 @@ void WeatherApp::run() {
canvas->print(String(temp, 1) + " °C");
canvas->setFont(FONT_10x20);
canvas->setCursor(canvas->width() / 2, canvas->height() / 2 + 20 / 2 + 10);
canvas->print(String(wind, 1) + " м/с");
canvas->print(String(wind, 1) + " км/год");
Copy link
Collaborator

Choose a reason for hiding this comment

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

додаткові зміни бажано робити окремими пул реквестами

@noisedsn noisedsn requested a review from frostmorn December 25, 2024 07:57
}

lilka::MenuItem item;
menu.getItem(index, &item);
Copy link
Collaborator

Choose a reason for hiding this comment

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

припускаю тестував як це працює і забув видалити :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Майже. Копіпастив з доки приклад і не все видалив :)

@@ -118,6 +118,9 @@ void LilTrackerApp::run() {

char str[64];

// Set initial volume
sequencer.setMasterVolume(0.25f * lilka::audio.volumeLevel / 100);
Copy link
Collaborator

Choose a reason for hiding this comment

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

в принципі це непогано.
зазвичай поля класу в яких знаходяться якісь дані які можуть змінюватися роблять приватними,
і реалізують для них два методи, типу
getVolume()
і
setVolume()
на перспективу це дозволяє потім не шукати по всьому коду де саме volume змінюється, та дозволяє відносно швидко вирішити проблему читання\запису данних з різних потоків.
хоча це і майже неможливий сценарій в нашому випадку :)
тому дані зазвичай private


lilka::Menu menu("Звук");
menu.addActivationButton(lilka::Button::B);
menu.addItem("Гучність:", 0, 0U, static_cast<String>(lilka::audio.volumeLevel));
Copy link
Collaborator

Choose a reason for hiding this comment

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

можна просто String(lilka::audio.volumeLevel)


// lilka::audio.updateSettings();

return;
Copy link
Collaborator

Choose a reason for hiding this comment

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

це відбудеться саме по собі, return можна і не писати :)

взагалі, зазвичай return використовують для повернення якихось значень, або якщо хочеться вийти раніше ніж заплановано. тут ти вийдеш з функції в будь-якому випадку

void SoundConfigApp::run() {
Preferences prefs;
prefs.begin("sound", false);
lilka::audio.volumeLevel = prefs.getUInt("volumeLevel", 100);
Copy link
Collaborator

@frostmorn frostmorn Dec 25, 2024

Choose a reason for hiding this comment

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

в принципі, можна перенести startupSound, startupBuzzer і volumeLevel в приватні поля audio,
реалізувати для них публічні методи
bool isStartupSoundEnabled/setStartupSoundEnabled(bool)
bool isStartupBuzzerEnabled/setStartupBuzzerEnabled(bool)
void setVolumeLevel(uint32_t)
uint32_t getVolumeLevel(uint32_t)

підгрузку початкових значень запхнути в audio.begin

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants