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

Нова сумісність: ESP32-S3, ESP32-C3 #320

Merged
merged 31 commits into from
Feb 10, 2025
Merged

Conversation

v00g100skr
Copy link
Member

@v00g100skr v00g100skr commented Feb 4, 2025

Summary by CodeRabbit

  • Нова функціональність

    • Запроваджено нові варіанти прошивки (lite, test, telnet, а також для ESP32-S3 та ESP32-C3), що розширюють підтримку апаратних платформ та дозволяють вибрати оптимальну конфігурацію для вашого пристрою.
    • Оновлено ключові бібліотеки, що покращує стабільність та продуктивність роботи прошивки.
    • Додано підтримку нових чіпів ESP32-S3 та ESP32-C3 у документації та конфігураціях прошивки.
    • Вдосконалено механізм логування, що дозволяє більш гнучко налаштовувати параметри логування.
  • Робочі процеси

    • Оптимізовано автоматичну збірку та реліз оновлень, що спрощує доставку та встановлення нових версій прошивки.
    • Вдосконалено логіку обробки даних у сервері WebSocket для кращого управління біном.

@v00g100skr v00g100skr added the покращення прошивки This will not be worked on label Feb 4, 2025
@v00g100skr v00g100skr requested a review from Foroxon February 4, 2025 17:19
Copy link
Contributor

coderabbitai bot commented Feb 4, 2025

Walkthrough

Внесено суттєві зміни в конфігурацію платформи, ініціалізацію пінів, логування та збірку прошивки. Оновлено файл platformio.ini із додаванням нових середовищ (lite, test, telnet, esp32s3, esp32c3) та поновлено залежності бібліотек. Змінено логіку ініціалізації пінів і налаштування LED-стрічки, а також оновлено запуск і компіляцію через GitHub Actions із введенням нового параметра project-env. Додано нові асинхронні функції у сервері оновлень та розширено можливості управління бінарними файлами через вебсокети. Видалено застарілі оголошення з Definitions.h.

Changes

Файл(и) Опис змін
firmware/platformio.ini Додано секцію [platformio] з default_envs = firmware; оновлено [env:firmware] (оновлено платформу та бібліотеку FastLED, додано флоги); додано нові середовища: firmware_lite, firmware_test, firmware_telnet, firmware_esp32s3, firmware_esp32c3 із відповідними налаштуваннями та платами.
firmware/src/JaamFirmware.cpp Змінено функції initLegacy та initFastledStrip: оновлено логіку збереження пінів, додано новий зарезервований пін; використання макросу GENERATE_PIN_CASE для додавання підтримки визначених пінів; додано перевірку підтримуваних LED-пінів з логуванням помилок.
firmware/src/JaamSettings.cpp Ініціалізаційні значення пінів (POWER_PIN, WIFI_PIN, DATA_PIN, HA_PIN, RESERVED_PIN) змінено на -1, що вказує на неактивний або незаданний стан.
firmware/src/Constants.h Видалено включення бібліотеки TelnetSpy; додано умовну компіляцію для визначення підтримуваних LED-пінів залежно від платформи; визначено макрос GENERATE_PIN_CASE; оновлено кількість опцій для legacy режимів.
.github/workflows/*.yml (beta_release, compile, release, upload-pages) Перероблено кроки GitHub Actions: оновлено параметри для середовищ збірки через project-env, додано нові завдання для компіляції прошивок для esp32s3, esp32c3 та telnet; змінено кроки оновлення версій в platformio.ini; розширено логіку копіювання та завантаження бінарних файлів.
.github/workflows/firmware-compile/action.yml Додано вхідний параметр project-env та оновлено команду побудови з урахуванням цього параметра (pio run -d ${{ inputs.project-folder }} -e ${{ inputs.project-env }}).
deploy/redeploy_update_server.sh Додано нові змінні SHARED_BETA_S3_PATH та SHARED_BETA_C3_PATH для розширення можливостей конфігурації.
deploy/update_server/update_server.py Додано асинхронні функції update_board та update_beta_board для обслуговування файлів прошивки; видалено функції spiffs_update та spiffs_update_beta; розширено функцію update_cache для роботи з новими каталогами s3 та c3.
deploy/websocket_server/websocket_server.py Додано атрибути s3_bins, s3_test_bins, c3_bins, c3_test_bins у класі SharedData; оновлено функції alerts_data та update_shared_data для обробки нових типів бінів із memcached.
Інші файли прошивки (JaamClimateSensor, JaamDisplay, JaamHomeAssistant, JaamLightSensor, JaamLogs, JaamUtils) Внесено зміни включень заголовков: замінено Constants.h/Definitions.h на JaamLogs.h; у JaamUtils.h рефакторинг функції fillFwVersion з використанням std::string замість C-рядків.
Файли flasher (index.html, manifest-beta.json) Оновлено опис підтримуваних чипів для JAAM Beta у HTML; додано нові конфігурації збірки для ESP32-S3 та ESP32-C3 у manifest-beta.json з відповідними шляхами та зміщеннями для бінарних файлів.

Sequence Diagram(s)

sequenceDiagram
    participant Dev as Розробник
    participant GA as GitHub Actions
    participant C as Завдання компіляції
    participant A as Артефакти
    participant R as Завдання релізу

    Dev->>GA: Надсилання змін (push)
    GA->>C: Запуск завдань компіляції (firmware, esp32s3, esp32c3, telnet, lite, test)
    C->>C: Виконання команди `pio run` з параметром `project-env`
    C->>A: Створення та завантаження артефактів (бінарні файли)
    A->>R: Передача артефактів для релізу
    R->>GA: Оновлення файлу `platformio.ini` та фіксація змін
Loading
sequenceDiagram
    participant Client as Клієнт
    participant US as Update Server

    Client->>US: Запит на оновлення прошивки (стандарт/бета)
    alt Стандартний запит
      US->>US: Виклик функції update_board()
    else Бета запит
      US->>US: Виклик функції update_beta_board()
    end
    US->>Client: Відправка запитаного файлу
Loading

Possibly related PRs

Suggested labels

технічне

Suggested reviewers

  • v00g100skr

Poem

Я кролик, в тривозі коду,
Радію змінам від серця і носу,
Нова логіка і збірка – мов зоря,
Піни, фірмвари зливаються в єдину хоробру строю,
Бінарки сяють, мов морквяний промінь в небі синьому,
В світі коду – я святкую зміни з посмішкою!
🥕🚀


📜 Recent review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 130619d and eaf1b1d.

📒 Files selected for processing (2)
  • firmware/platformio.ini (2 hunks)
  • flasher/index.html (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • flasher/index.html
⏰ Context from checks skipped due to timeout of 90000ms (6)
  • GitHub Check: compile_jaam2_test
  • GitHub Check: compile_firmware_telnet
  • GitHub Check: compile_firmware_lite
  • GitHub Check: compile_firmware_esp32c3
  • GitHub Check: compile_firmware_esp32s3
  • GitHub Check: compile_firmware
🔇 Additional comments (11)
firmware/platformio.ini (11)

11-13: Додавання нового розділу [platformio]
Налаштування за умовчанням через default_envs = firmware виглядає коректним та забезпечує зручність для збірки. Переконайтеся, що CI/CD процес та всі інші конфігурації враховують це значення.


15-15: Оновлення платформи
Версія платформи оновлена до espressif32@6.10.0, що відповідає сучасним вимогам і, ймовірно, забезпечує підтримку нових плат, таких як ESP32-S3 та ESP32-C3. Рекомендується перевірити сумісність цієї версії з існуючими модулями прошивки.


17-18: Налаштування build_src_flags
Визначення прапорця -D VERSION="4.3.1" встановлено як джерело версії прошивки. Це коректне рішення, але слід впевнитися, що ця версія відповідає поточним релізним вимогам проекту.


19-30: Конфігурація build_flags та інших параметрів
Набір прапорців у build_flags (наприклад, ARDUINO_OTA_ENABLED, FW_UPDATE_ENABLED, HA_ENABLED, DISPLAY_ENABLED тощо) чітко відображає необхідні можливості прошивки. Будь ласка, перевірте, що всі ці прапорці відповідають апаратним вимогам і бізнес-логіці проекту.


32-46: Оновлення бібліотечних залежностей
Оновлено бібліотеку FastLED до версії 3.9.13 та додано/оновлено залежність для melody-player (з вказаною версією 2.4.0). Переконайтеся, що ці оновлення не впливають на сумісність з іншими бібліотеками та апаратним забезпеченням.


50-63: Новий environment [env:firmware_lite]
Конфігурація firmware_lite створена для спрощеної версії прошивки із відключенням ряду функцій (OTA, FW_UPDATE, HA, сенсори тощо). Переконайтеся, що це відповідає вимогам для легких версій пристроїв, де обмежені ресурси або непотрібна повна функціональність.


64-77: Новий environment [env:firmware_test]
Налаштування firmware_test включає режим тестування із необхідними функціональними прапорцями, що дозволяє проводити тестування окремих компонентів (дисплей, датчики). Переконайтеся, що ця конфігурація відповідає сценаріям валідації прошивки.


78-91: Новий environment [env:firmware_telnet]
Конфігурація firmware_telnet активує Telnet для віддаленого доступу, зберігаючи при цьому актуальну підтримку інших функцій. Важливо переконатися, що відключення OTA не спричинить небажаних конфліктів із Telnet-підключенням.


92-95: Новий environment [env:firmware_esp32s3]
Середовище для плати ESP32-S3 (esp32-s3-devkitc-1) створене шляхом розширення базової конфігурації. Переконайтеся, що специфічні налаштування для цієї плати коректно наслідуються від env:firmware.


96-109: Новий environment [env:firmware_esp32c3]
Конфігурація для ESP32-C3 (esp32-c3-devkitc-02) містить унікальний набір прапорців. Рекомендується перевірити, чи всі задані прапорці враховують особливості апаратної платформи ESP32-C3, що може відрізнятись від інших моделей.


110-113: Новий environment [env:firmware_exception_decoder]
Додавання середовища для декодування винятків за допомогою моніторингового фільтру esp32_exception_decoder є корисним для відлагодження. Це дозволяє оперативно аналізувати помилки під час роботи пристроїв.


Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media?

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (Invoked using PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai generate docstrings to generate docstrings for this PR. (Beta)
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

🧹 Nitpick comments (2)
firmware/src/JaamSettings.cpp (1)

44-48: Схвалено зміну значень пінів за замовчуванням!

Встановлення значення -1 для пінів POWER, WIFI, DATA, HA та RESERVED є правильним рішенням, оскільки це:

  • Запобігає випадковому використанню неініціалізованих пінів
  • Робить явною необхідність конфігурації пінів
  • Покращує сумісність з різними апаратними платформами, включаючи ESP32-S3

Рекомендую додати коментар у код, який пояснює, що значення -1 означає "пін не налаштовано". Наприклад:

 std::map<Type, SettingItemInt> intSettings = {
+    // Pin values of -1 indicate unconfigured pins that must be set via configuration
     {POWER_PIN, {"powp", -1}},
     {WIFI_PIN, {"wifip", -1}},
     {DATA_PIN, {"datap", -1}},
     {HA_PIN, {"hap", -1}},
     {RESERVED_PIN, {"resp", -1}},
firmware/src/JaamFirmware.cpp (1)

429-436: Додайте валідацію та коментарі для призначень пінів.

Рекомендується:

  1. Додати валідацію значень пінів перед збереженням
  2. Додати коментарі, що пояснюють призначення кожного піна
  3. Обробляти помилки при некоректних значеннях пінів
 LOG.println("Mode: jaam 1");    

+// Validate pin numbers
+if (12 < 0 || 14 < 0 || 25 < 0 || 26 < 0 || 27 < 0) {
+  LOG.println("Error: Invalid pin numbers");
+  return;
+}

+// Save pin assignments to settings
+// POWER pin - controls power indicator LED
 settings.saveInt(POWER_PIN, 12, false);
+// WIFI pin - indicates WiFi connection status  
 settings.saveInt(WIFI_PIN, 14, false);
+// DATA pin - shows data connection status
 settings.saveInt(DATA_PIN, 25, false);
+// HA pin - indicates Home Assistant connection
 settings.saveInt(HA_PIN, 26, false);
+// RESERVED pin - for future use
 settings.saveInt(RESERVED_PIN, 27, false);
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 79fefd0 and f953f2b.

📒 Files selected for processing (3)
  • firmware/platformio.ini (1 hunks)
  • firmware/src/JaamFirmware.cpp (2 hunks)
  • firmware/src/JaamSettings.cpp (1 hunks)
✅ Files skipped from review due to trivial changes (1)
  • firmware/platformio.ini
⏰ Context from checks skipped due to timeout of 90000ms (3)
  • GitHub Check: compile_jaam2_test
  • GitHub Check: compile_firmware
  • GitHub Check: compile_firmware_lite
🔇 Additional comments (1)
firmware/src/JaamFirmware.cpp (1)

448-454: LGTM! Коректна послідовність ініціалізації пінів.

Послідовність ініціалізації логічна:

  1. Встановлення режимів пінів
  2. Активація індикатора живлення

firmware/platformio.ini Outdated Show resolved Hide resolved
firmware/src/JaamFirmware.cpp Outdated Show resolved Hide resolved
@@ -3426,15 +3426,13 @@ void initLegacy() {
#endif
switch (settings.getInt(LEGACY)) {
case 0:
LOG.println("Mode: jaam 1");
LOG.println("Mode: jaam 1");
Copy link
Member

Choose a reason for hiding this comment

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

лишні пробіли в кінці рядка

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 2

🧹 Nitpick comments (1)
firmware/src/Constants.h (1)

341-342: Рекомендується додати обґрунтування для розміру JSON.

Константа MAX_JSON_SIZE встановлена як 6KB без пояснення. Додайте коментар, що пояснює, чому обрано саме це значення та які типи даних воно має вмістити.

-static const size_t MAX_JSON_SIZE = 6000; // 6KB
+static const size_t MAX_JSON_SIZE = 6000; // 6KB - максимальний розмір для зберігання налаштувань, статусів регіонів та метаданих
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 9130b8f and ae9e7f0.

📒 Files selected for processing (2)
  • firmware/src/Constants.h (1 hunks)
  • firmware/src/JaamFirmware.cpp (3 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • firmware/src/JaamFirmware.cpp
⏰ Context from checks skipped due to timeout of 90000ms (3)
  • GitHub Check: compile_firmware_lite
  • GitHub Check: compile_firmware
  • GitHub Check: compile_jaam2_test

firmware/src/Constants.h Show resolved Hide resolved
firmware/src/Constants.h Outdated Show resolved Hide resolved
@v00g100skr v00g100skr self-assigned this Feb 4, 2025
@v00g100skr v00g100skr marked this pull request as draft February 4, 2025 22:15
deploy/websocket_server/websocket_server.py Dismissed Show dismissed Hide dismissed
deploy/websocket_server/websocket_server.py Dismissed Show dismissed Hide dismissed
deploy/websocket_server/websocket_server.py Dismissed Show dismissed Hide dismissed
deploy/websocket_server/websocket_server.py Dismissed Show dismissed Hide dismissed
@v00g100skr v00g100skr linked an issue Feb 8, 2025 that may be closed by this pull request
@v00g100skr v00g100skr changed the title Нова сумісність: ESP32-S3 Нова сумісність: ESP32-S3, ESP32-C3, Feb 8, 2025
# Conflicts:
#	.github/workflows/upload-pages/action.yml
#	firmware/src/JaamFirmware.cpp
@Foroxon Foroxon marked this pull request as ready for review February 10, 2025 12:41
# Conflicts:
#	firmware/src/JaamFirmware.cpp
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 3

🔭 Outside diff range comments (1)
.github/workflows/release.yml (1)

132-145: ** Перевірте правильність шляхів при копіюванні файлів**
Команди копіювання видаються підозрілими – виглядає, що імена файлів вказані двічі. Наприклад:

- cp -f ${{ github.workspace }}/JAAM_${{ inputs.release-version }}.bin/JAAM_${{ inputs.release-version }}.bin ${{ github.workspace }}/bin/${{ inputs.release-version }}.bin
+ cp -f ${{ github.workspace }}/JAAM_${{ inputs.release-version }}.bin ${{ github.workspace }}/bin/${{ inputs.release-version }}.bin

Перевірте, чи правильно вказаний шлях до файлу (чи це файл, а не папка), і виправте дублікат імені, якщо потрібно.

♻️ Duplicate comments (3)
deploy/update_server/update_server.py (1)

167-168: Додати перевірку існування файлу.

Функція update_beta_board також потребує перевірки наявності файлу.

Пропоную додати аналогічну перевірку:

async def update_beta_board(request):
+    filepath = f'{shared_beta_path}/{request.path_params["board"]}/{request.path_params["filename"]}.bin'
+    if not os.path.isfile(filepath):
+        raise HTTPException(status_code=404, detail="File not found")
-    return FileResponse(f'{shared_beta_path}/{request.path_params["board"]}/{request.path_params["filename"]}.bin')
+    return FileResponse(filepath)
firmware/src/Constants.h (2)

334-343: Покращити визначення підтримуваних платформ.

Поточна реалізація не включає перевірку версії ESP-IDF.

Пропоную розширити перевірки:

-#if ARDUINO_ESP32_DEV
+#if defined(ARDUINO_ESP32_DEV) && ESP_IDF_VERSION >= ESP_IDF_VERSION_VAL(4, 0, 0)
     #define SUPPORTED_LEDS_PINS {2, 4, 12, 13, 14, 15, 16, 17, 18, 25, 26, 27, 32, 33}
-#elif ARDUINO_ESP32S3_DEV
+#elif defined(ARDUINO_ESP32S3_DEV) && ESP_IDF_VERSION >= ESP_IDF_VERSION_VAL(4, 4, 0)
     #define SUPPORTED_LEDS_PINS {2, 4, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19, 20, 21}
-#elif ARDUINO_ESP32C3_DEV
+#elif defined(ARDUINO_ESP32C3_DEV) && ESP_IDF_VERSION >= ESP_IDF_VERSION_VAL(4, 4, 0)
     #define SUPPORTED_LEDS_PINS {2, 4, 18}
 #else
-    #error "Платформа не підтримується!"
+    #error "Підтримуються лише ESP32 (IDF v4.0+), ESP32-S3 та ESP32-C3 (IDF v4.4+)"
 #endif

345-347: Додати перевірку валідності піна.

Макрос GENERATE_PIN_CASE не перевіряє підтримку піна на поточній платформі.

Пропоную додати перевірку:

+// Допоміжна функція для перевірки підтримки піна
+constexpr bool isPinSupported(int pin) {
+    constexpr int supported_pins[] = SUPPORTED_LEDS_PINS;
+    for (auto supported_pin : supported_pins) {
+        if (pin == supported_pin) return true;
+    }
+    return false;
+}

-#define GENERATE_PIN_CASE(pin) \
-    case pin: FastLED.addLeds<NEOPIXEL, pin>(const_cast<CRGB*>(leds), pixelcount); break;
+#define GENERATE_PIN_CASE(pin) \
+    case pin: \
+        static_assert(isPinSupported(pin), "LED пін не підтримується на цій платформі"); \
+        FastLED.addLeds<NEOPIXEL, pin>(const_cast<CRGB*>(leds), pixelcount); \
+        break;
🧹 Nitpick comments (8)
deploy/websocket_server/websocket_server.py (2)

374-425: Рефакторинг для зменшення дублювання коду

Код для обробки бінарних файлів дублюється для кожного типу прошивки. Рекомендується винести спільну логіку в окрему функцію.

Пропоную створити допоміжну функцію:

async def send_bins_update(websocket, client, shared_data, firmware_type, is_test=False):
    bins_attr = f"{firmware_type}_bins" if firmware_type else "bins"
    test_suffix = "test_" if is_test else ""
    shared_bins = getattr(shared_data, f"{test_suffix}{bins_attr}")
    
    if client[f"{test_suffix}bins"] != shared_bins:
        temp_bins = list(json.loads(shared_bins))
        if not firmware_type and (firmware.startswith("3.") or firmware.startswith("2.") or firmware.startswith("1.")):
            temp_bins = list(filter(lambda bin: not bin.startswith("4."), temp_bins))
            temp_bins.append(f"latest{'_beta' if is_test else ''}.bin")
        temp_bins.sort(key=bin_sort, reverse=True)
        
        payload_type = "test_bins" if is_test else "bins"
        payload = f'{{"payload": "{payload_type}", "{payload_type}": {temp_bins}}}'
        await websocket.send(payload)
        logger.debug(f"{client_ip}:{chip_id} <<< new {test_suffix}{firmware_type}_bins")
        client[f"{test_suffix}bins"] = shared_bins

870-889: Спростіть код за допомогою тернарних операторів

Для покращення читабельності коду рекомендується використати тернарні оператори замість блоків if-else.

Застосуйте цей diff:

-    if s3_bins_cached:
-        s3_bins_cached_data = s3_bins_cached.decode("utf-8")
-    else:
-        s3_bins_cached_data = "[]"
+    s3_bins_cached_data = s3_bins_cached.decode("utf-8") if s3_bins_cached else "[]"

-    if s3_test_bins_cached:
-        s3_test_bins_cached_data = s3_test_bins_cached.decode("utf-8")
-    else:
-        s3_test_bins_cached_data = "[]"
+    s3_test_bins_cached_data = s3_test_bins_cached.decode("utf-8") if s3_test_bins_cached else "[]"

-    if c3_bins_cached:
-        c3_bins_cached_data = c3_bins_cached.decode("utf-8")
-    else:
-        c3_bins_cached_data = "[]"
+    c3_bins_cached_data = c3_bins_cached.decode("utf-8") if c3_bins_cached else "[]"

-    if c3_test_bins_cached:
-        c3_test_bins_cached_data = c3_test_bins_cached.decode("utf-8")
-    else:
-        c3_test_bins_cached_data = "[]"
+    c3_test_bins_cached_data = c3_test_bins_cached.decode("utf-8") if c3_test_bins_cached else "[]"
🧰 Tools
🪛 Ruff (0.8.2)

870-873: Use ternary operator s3_bins_cached_data = s3_bins_cached.decode("utf-8") if s3_bins_cached else "[]" instead of if-else-block

Replace if-else-block with s3_bins_cached_data = s3_bins_cached.decode("utf-8") if s3_bins_cached else "[]"

(SIM108)


880-883: Use ternary operator c3_bins_cached_data = c3_bins_cached.decode("utf-8") if c3_bins_cached else "[]" instead of if-else-block

Replace if-else-block with c3_bins_cached_data = c3_bins_cached.decode("utf-8") if c3_bins_cached else "[]"

(SIM108)

firmware/src/JaamLogs.h (1)

1-8: Додати перевірку ініціалізації.

Поточна реалізація не перевіряє, чи була ініціалізована SerialAndTelnet перед використанням. Це може призвести до невизначеної поведінки.

Пропоную додати перевірку ініціалізації:

#if TELNET_ENABLED
#include <TelnetSpy.h>

static TelnetSpy SerialAndTelnet;
-#define LOG SerialAndTelnet
+#define LOG (SerialAndTelnet.isInitialized() ? SerialAndTelnet : Serial)
#else
#define LOG Serial
#endif
deploy/update_server/update_server.py (1)

187-228: Оптимізувати обробку файлів.

Код для обробки файлів дублюється для кожного типу плати.

Пропоную створити допоміжну функцію:

+def get_sorted_files(path, key=bin_sort, reverse=True):
+    return sorted(
+        [
+            file
+            for file in os.listdir(path)
+            if (os.path.isfile(os.path.join(path, file)) and file.endswith(".bin"))
+        ],
+        key=key,
+        reverse=reverse,
+    )

async def update_cache():
    mc = Client(memcached_host, memcached_port)
-    filenames = sorted(
-        [
-            file
-            for file in os.listdir(shared_path)
-            if (os.path.isfile(os.path.join(shared_path, file)) and file.endswith(".bin"))
-        ]
-    )
+    filenames = get_sorted_files(shared_path, reverse=False)
    # ... аналогічно для інших викликів
firmware/src/JaamUtils.h (1)

42-68: Покращити обробку версій.

Функція fillFwVersion не валідує вхідні дані та не має обмежень на довжину версії.

Пропоную додати валідацію:

static void fillFwVersion(char* result, Firmware firmware) {
+  // Валідація вхідних даних
+  if (firmware.major < 0 || firmware.minor < 0 || firmware.patch < 0 || firmware.betaBuild < 0) {
+    strcpy(result, "invalid");
+    return;
+  }

  std::string version = std::to_string(firmware.major) + "." + std::to_string(firmware.minor);

  if (firmware.patch > 0) {
    version += "." + std::to_string(firmware.patch);
  }

  if (firmware.isBeta) {
    version += "-b" + std::to_string(firmware.betaBuild);
  }

+  // Перевірка максимальної довжини
+  const size_t MAX_VERSION_LENGTH = 32;
+  if (version.length() >= MAX_VERSION_LENGTH) {
+    strcpy(result, "too_long");
+    return;
+  }

  strcpy(result, version.c_str());
}
firmware/src/JaamFirmware.cpp (1)

3586-3590: Додайте повідомлення про помилку для неправильної платформи.

У разі неправильної платформи, код повинен виводити більш інформативне повідомлення про помилку.

Застосуйте цей diff:

     default:
-        LOG.print("Error: Unexpected pin configuration for this board: ");
-        LOG.println(pin);
+        LOG.printf("Error: Unsupported board type or pin %d. Supported boards: ESP32-DEV, ESP32-S3, ESP32-C3\n", pin);
         break;
deploy/redeploy_update_server.sh (1)

6-7: Додавання нових змінних для шляхів бета-платформ:
Змінні SHARED_BETA_S3_PATH та SHARED_BETA_C3_PATH додано для розширення можливостей налаштування спільних шляхів. Проте, наразі ці змінні не використовуються у логіці скрипту. Рекомендую перевірити їх призначення або додати відповідні коментарі для допомоги майбутнім розробникам.

🧰 Tools
🪛 Shellcheck (0.10.0)

[warning] 6-6: SHARED_BETA_S3_PATH appears unused. Verify use (or export if used externally).

(SC2034)


[warning] 7-7: SHARED_BETA_C3_PATH appears unused. Verify use (or export if used externally).

(SC2034)

.github/workflows/upload-pages/action.yml (1)

42-42: ** Вирівнювання відступів та форматування**
Зауважте, що YAMLlint вказує на помилки форматування:

  • Рядок 42: неправильний відступ – очікується 10 пробілів, знайдено 12.
  • Рядок 54: аналогічна проблема з відступом.
  • Рядок 70: виявлено зайві пробіли в кінці рядка.
    Рекомендується виправити ці незначні стилістичні недоліки для підтримання чистоти YAML-файлу.

Also applies to: 54-54, 70-70

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between ae9e7f0 and 6798c74.

⛔ Files ignored due to path filters (4)
  • flasher/bins/jaam.bootloader.c3.bin is excluded by !**/*.bin
  • flasher/bins/jaam.bootloader.s3.bin is excluded by !**/*.bin
  • flasher/bins/jaam.partitions.c3.bin is excluded by !**/*.bin
  • flasher/bins/jaam.partitions.s3.bin is excluded by !**/*.bin
📒 Files selected for processing (25)
  • .github/workflows/beta_release.yml (2 hunks)
  • .github/workflows/compile.yml (3 hunks)
  • .github/workflows/firmware-compile/action.yml (2 hunks)
  • .github/workflows/release.yml (5 hunks)
  • .github/workflows/upload-pages/action.yml (3 hunks)
  • deploy/redeploy_update_server.sh (1 hunks)
  • deploy/update_server/update_server.py (4 hunks)
  • deploy/websocket_server/websocket_server.py (10 hunks)
  • firmware/platformio.ini (2 hunks)
  • firmware/src/Constants.h (1 hunks)
  • firmware/src/Definitions.h (0 hunks)
  • firmware/src/JaamClimateSensor.cpp (1 hunks)
  • firmware/src/JaamClimateSensor.h (0 hunks)
  • firmware/src/JaamDisplay.cpp (1 hunks)
  • firmware/src/JaamDisplay.h (0 hunks)
  • firmware/src/JaamFirmware.cpp (4 hunks)
  • firmware/src/JaamHomeAssistant.h (0 hunks)
  • firmware/src/JaamLightSensor.cpp (1 hunks)
  • firmware/src/JaamLightSensor.h (0 hunks)
  • firmware/src/JaamLogs.h (1 hunks)
  • firmware/src/JaamUtils.h (2 hunks)
  • flasher/index.html (1 hunks)
  • flasher/manifest-beta-c3.json (1 hunks)
  • flasher/manifest-beta-s3.json (1 hunks)
  • flasher/manifest-beta.json (1 hunks)
💤 Files with no reviewable changes (5)
  • firmware/src/JaamLightSensor.h
  • firmware/src/JaamClimateSensor.h
  • firmware/src/JaamHomeAssistant.h
  • firmware/src/JaamDisplay.h
  • firmware/src/Definitions.h
✅ Files skipped from review due to trivial changes (3)
  • firmware/src/JaamClimateSensor.cpp
  • firmware/src/JaamLightSensor.cpp
  • firmware/src/JaamDisplay.cpp
🧰 Additional context used
🪛 Shellcheck (0.10.0)
deploy/redeploy_update_server.sh

[warning] 6-6: SHARED_BETA_S3_PATH appears unused. Verify use (or export if used externally).

(SC2034)


[warning] 7-7: SHARED_BETA_C3_PATH appears unused. Verify use (or export if used externally).

(SC2034)

🪛 YAMLlint (1.35.1)
.github/workflows/beta_release.yml

[warning] 42-42: wrong indentation: expected 10 but found 12

(indentation)


[warning] 54-54: wrong indentation: expected 10 but found 12

(indentation)


[error] 70-70: trailing spaces

(trailing-spaces)

🪛 Ruff (0.8.2)
deploy/websocket_server/websocket_server.py

870-873: Use ternary operator s3_bins_cached_data = s3_bins_cached.decode("utf-8") if s3_bins_cached else "[]" instead of if-else-block

Replace if-else-block with s3_bins_cached_data = s3_bins_cached.decode("utf-8") if s3_bins_cached else "[]"

(SIM108)


880-883: Use ternary operator c3_bins_cached_data = c3_bins_cached.decode("utf-8") if c3_bins_cached else "[]" instead of if-else-block

Replace if-else-block with c3_bins_cached_data = c3_bins_cached.decode("utf-8") if c3_bins_cached else "[]"

(SIM108)

🔇 Additional comments (53)
deploy/websocket_server/websocket_server.py (3)

80-83: Додавання атрибутів для ESP32-S3 та ESP32-C3 виглядає добре!

Ініціалізація нових атрибутів відповідає існуючому шаблону та узгоджується з логікою коду.


677-704: Обробка помилок для S3 та C3 бінарних файлів реалізована коректно!

Код відповідає існуючому шаблону обробки помилок та використовує послідовний формат логування.


403-403: Зверніть увагу на безпеку логування

Логування ідентифікаторів чіпів може розкривати конфіденційну інформацію. Рекомендується маскувати або видаляти чутливі дані перед логуванням.

Also applies to: 410-410, 417-417, 424-424

firmware/src/JaamFirmware.cpp (2)

3532-3547: LGTM! Перевірка підтримуваних пінів.

Хороша реалізація перевірки підтримуваних пінів перед ініціалізацією LED-стрічки. Це допоможе запобігти використанню неправильних пінів.


3549-3585: LGTM! Підтримка різних платформ ESP32.

Правильна реалізація підтримки різних платформ ESP32 з використанням препроцесорних директив. Кожна платформа має свій власний набір підтримуваних пінів.

flasher/manifest-beta-s3.json (2)

1-7: Перевірка метаданих файлу:
Метадані (ім'я, версія, URL фінансування, параметри установки) для JAAM S3 Beta задані коректно та відповідають очікуваній структурі.


7-17: Перевірка блоку "builds":
Блок "builds" для ESP32-S3 має правильну структуру. Шляхи до бінарних файлів і зсуви вказані коректно, що дозволяє забезпечити належну завантажувальну послідовність.

flasher/manifest-beta-c3.json (2)

1-6: Перевірка метаданих файлу:
Метадані для JAAM C3 Beta (назва, версія, URL фінансування, параметри установки) заповнені правильно і відповідають вимогам до нового маніфесту.


7-17: Перевірка блоку "builds":
Конфігурація збірки для ESP32-C3 оформлена коректно: всі необхідні шляхи та зміщення для бінарних файлів вказані, що забезпечує правильну роботу для цієї платформи.

.github/workflows/firmware-compile/action.yml (2)

7-9: Додавання параметра project-env:
Новий вхідний параметр project-env (опис: "Project environment to build", значення за замовчуванням: "firmware") успішно інтегровано для налаштування збірки в різних середовищах.


30-30: Оновлена команда збірки:
Команда на рядку 30 коректно модифікована для використання нового параметра -e ${{ inputs.project-env }}, що дозволяє будувати проекти відповідно до обраного середовища.

flasher/manifest-beta.json (2)

17-27: Конфігурація для ESP32-S3:
Новий блок конфігурації для ESP32-S3 додано з вірно вказаними шляхами до бінарних файлів і зсувами. Структура відповідає загальному формату маніфесту.


28-36: Конфігурація для ESP32-C3:
Додано окрему конфігурацію для ESP32-C3 з правильними параметрами (chipFamily, improv, parts). Зміни сумісні з існуючою структурою та спрямовані на розширення підтримки нових платформ.

firmware/platformio.ini (8)

11-13: ** Додавання розділу [platformio] та встановлення default_envs**
Новий розділ [platformio] із встановленням default_envs = firmware виглядає слушно. Переконайтеся, що дане налаштування охоплює всі необхідні середовища збірки.


15-18: ** Оновлення платформи до espressif32@6.10.0 та базових прапорців**
Оновлення рядка platform = espressif32@6.10.0 забезпечує використання актуальнішої версії. Ретельно перевірте сумісність із залученими бібліотеками та апаратним забезпеченням.


32-47: ** Оновлення залежностей бібліотек**
Версія FastLED оновлена до 3.9.13, а також додано залежність на melody-player (версія 2.4.0). Переконайтеся, що ці версії відповідають вимогам проекту та не порушують сумісність.


50-63: ** Додавання оточення firmware_lite**
Новий environment [env:firmware_lite] із вимкненими функціями (наприклад, OTA, FW_UPDATE, HA та іншими) налаштований коректно. Переконайтеся, що ці прапорці відповідають вимогам lite-версії.


64-77: ** Додавання оточення firmware_test**
Оточення [env:firmware_test], яке включає прапорець -D TEST_MODE, добре ізолює тестовий режим. Переконайтеся, що додаткові параметри тестування охоплені належним чином.


78-91: ** Додавання оточення firmware_telnet**
Налаштування для [env:firmware_telnet] з увімкненим TELNET забезпечують додаткові опції для відлагодження. Варто перевірити, що всі потрібні функції активуються коректно.


92-95: ** Додавання підтримки ESP32-S3**
Оточення [env:firmware_esp32s3] встановлює потрібну плату esp32-s3-devkitc-1. Рекомендується протестувати збірку для цієї плати та, за потреби, додати специфічні прапорці.


96-109: ** Додавання підтримки ESP32-C3**
Оточення [env:firmware_esp32c3] налаштовано з платою esp32-c3-devkitc-02 та відповідними прапорцями збірки. Переконайтеся, що конфігурація повністю відповідає вимогам цієї платформи.

.github/workflows/upload-pages/action.yml (7)

10-15: ** Додавання нових вхідних параметрів для beta_s3 та beta_c3**
Нові параметри beta_s3_binary_path та beta_c3_binary_path дозволяють вказати шляхи до додаткових бінарних файлів. Переконайтеся, що ці налаштування узгоджуються з структурою проекту.


33-37: ** Крок копіювання стандартного beta-бінарного файлу**
Крок із назвою "Copy beta bin to flasher" коректно копіює бінарний файл за умови, що змінна beta_binary_path не порожня.


38-42: ** Крок копіювання beta S3 бінарного файлу**
Доданий крок "Copy beta s3 bin to flasher" забезпечує копіювання файлу для ESP32-S3. Переконайтеся, що шлях збереження відповідає вашим вимогам.


43-47: ** Крок копіювання beta C3 бінарного файлу**
Крок "Copy beta c3 bin to flasher" забезпечує обробку файлу для ESP32-C3. Все виглядає коректно.


48-52: ** Крок копіювання lite бінарного файлу**
Крок для копіювання lite версії файлу реалізовано аналогічно до інших. Перевірте, що файли розташовано за правильними шляхами.


65-68: ** Оновлення шаблонів sed для beta-версії**
Застосування sed-команд для заміни версії та номера beta збірки виглядає логічним. Ретельно протестуйте регулярні вирази, щоб уникнути несподіваних результатів.


73-73: ** Оновлення шаблону sed для lite-версії**
Внесені зміни у sed-шаблон для lite версії виглядають коректно.

.github/workflows/beta_release.yml (7)

27-28: ** Оновлення версії прошивки через sed**
Використання команди sed для заміни версії у файлі firmware/platformio.ini замість попереднього JaamFirmware.cpp відповідає новій структурі.


39-43: ** Додавання кроку компіляції для ESP32-S3**
Новий крок "Compile firmware s3" із параметром project-env: firmware_esp32s3 коректно включає підтримку ESP32-S3.

🧰 Tools
🪛 YAMLlint (1.35.1)

[warning] 42-42: wrong indentation: expected 10 but found 12

(indentation)


44-47: ** Копіювання бінарних файлів для ESP32-S3**
Крок копіювання файлів для ESP32-S3 забезпечує правильне розміщення зібраного бінарного файлу.


48-50: ** Очищення старих бінарних файлів для ESP32-S3**
Крок "Leave only 10 recent BETA S3 builds" забезпечує утримання останніх зібрань для цієї платформи.


51-55: ** Додавання кроку компіляції для ESP32-C3**
Новий крок "Compile firmware c3" із параметром project-env: firmware_esp32c3 дозволяє зібрати прошивку для ESP32-C3.

🧰 Tools
🪛 YAMLlint (1.35.1)

[warning] 54-54: wrong indentation: expected 10 but found 12

(indentation)


56-60: ** Копіювання бінарних файлів для ESP32-C3**
Крок копіювання бінарного файлу для ESP32-C3 налаштований схожим чином, як і для S3.


61-62: ** Очищення старих бінарних файлів для ESP32-C3**
Крок "Leave only 10 recent BETA C3 builds" забезпечує утримання потрібної кількості зібрань для ESP32-C3.

.github/workflows/compile.yml (6)

26-42: ** Задача компіляції для ESP32-S3**
Новий блок завдання compile_firmware_esp32s3 використовує project-env: firmware_esp32s3 та копіює зібраний бінарний файл. Здається, все реалізовано коректно.


43-57: ** Задача компіляції для ESP32-C3**
Завдання compile_firmware_esp32c3 додає підтримку збірки для ESP32-C3 із належним копіюванням файлу.


58-73: ** Оновлення збірки lite-версії**
Оновлено задачу compile_firmware_lite із параметром project-env: firmware_lite. Це дозволяє налаштувати легку версію прошивки.


74-89: ** Оновлення тестової збірки з project-env: firmware_test**
Оновлене завдання compile_jaam2_test використовує параметр project-env: firmware_test, що відповідає новій конфігурації оточення.


90-105: ** Додавання збірки з підтримкою Telnet**
Нова задача compile_firmware_telnet включає збірку із підтримкою Telnet, що може бути корисним для відлагодження.


114-123: ** Оновлення задачі збірки оновлювача**
У завданні compile_updater параметр перейшов від project-folder: updater до project-env: updater. Це спрощує конфігурацію і узгоджується з іншими завданнями.

.github/workflows/release.yml (11)

21-23: ** Оновлення версії у platformio.ini для релізу**
Використання sed-команди для заміни версії в firmware/platformio.ini відповідає новій структурі проекту. Переконайтеся, що регулярний вираз коректно відображає формат версії.


34-54: ** Завдання build_s3 для ESP32-S3**
Завдання build_s3 реалізує збірку для ESP32-S3 з використанням project-env: firmware_esp32s3. Кроки копіювання та завантаження артефактів виглядають логічно.


55-75: ** Завдання build_c3 для ESP32-C3**
Новий блок build_c3 забезпечує збірку для ESP32-C3 через project-env: firmware_esp32c3 із відповідними діями копіювання та завантаження.


77-96: ** Оновлення завдання build_lite**
Секция build_lite тепер оновлює версію в platformio.ini і використовує project-env: firmware_lite для збірки lite-версії прошивки.


97-111: ** Завдання release_firmware: оновлення релізу**
У завданні release_firmware об’єднано потреби з build, build_s3, build_c3 і build_lite. Крок заміни версії повторно застосовується для platformio.ini. Все відповідає новій структурі.


112-126: ** Завантаження артефактів для всіх середовищ**
Окремі кроки завантаження (download) артефактів для основної, S3, C3 та lite збірок налаштовані коректно. Перевірте, що артефакти мають очікувані імена та шляхи.


146-153: ** Очищення старих збірок для PROD, BETA, S3 та C3**
Команди для видалення старих бінарних файлів виглядають логічно. Переконайтеся, що задається потрібна кількість залишкових файлів.


154-160: ** Крок завантаження flasher на GitHub Pages**
Крок "Upload flasher to Github Pages" оновлено відповідно до нової файлової структури. Переконайтеся, що всі вказані шляхи відповідають актуальному розташуванню файлів.


161-166: ** Оновлення кроку commit_changes**
Параметр file_pattern оновлено для включення platformio.ini та нових директорій прошивок. Це дозволить зафіксувати всі важливі зміни після релізу.


167-175: ** Крок створення релізу**
Крок створення релізу із зазначенням артефактів для основної та lite версії виглядає коректно. Переконайтеся, що список артефактів відповідає новій логіці збірки.


176-184: ** Злиття master у develop після релізу**
Крок "Merge master -> develop" автоматизує синхронізацію після релізу. Переконайтеся, що весь процес злиття відповідає вашим вимогам CI/CD.

flasher/index.html (1)

104-104: Оновлено опис "JAAM Beta" для підтримки нових платформ.
Опис прошивки оновлено таким чином, що тепер він чітко зазначає підтримку платформ ESP32, ESP32-S3 та ESP32-C3. Зміни відповідають поставленим цілям PR і оформлені коректно.

@@ -367,7 +371,7 @@ async def alerts_data(websocket: ServerConnection, client, client_id, client_ip,
await websocket.send(payload)
logger.debug(f"{client_ip}:{chip_id} <<< new weather")
client["weather"] = shared_data.weather_v1
if client["bins"] != shared_data.bins:
if client["bins"] != shared_data.bins and not firmware.contains("-s3") and not firmware.contains("-c3"):
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

Виправте синтаксичну помилку в перевірці типу прошивки!

Метод contains() не існує для рядків у Python. Замість цього використовуйте оператор in.

Застосуйте цей diff:

-if client["bins"] != shared_data.bins and not firmware.contains("-s3") and not firmware.contains("-c3"):
+if client["bins"] != shared_data.bins and "-s3" not in firmware and "-c3" not in firmware:

Also applies to: 386-386, 387-387

Comment on lines +136 to +137
async def update_board(request):
return FileResponse(f'{shared_path}/{request.path_params["board"]}/{request.path_params["filename"]}.bin')
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

Додати перевірку існування файлу.

Функція update_board не перевіряє наявність файлу перед спробою його відправки.

Пропоную додати перевірку:

async def update_board(request):
+    filepath = f'{shared_path}/{request.path_params["board"]}/{request.path_params["filename"]}.bin'
+    if not os.path.isfile(filepath):
+        raise HTTPException(status_code=404, detail="File not found")
-    return FileResponse(f'{shared_path}/{request.path_params["board"]}/{request.path_params["filename"]}.bin')
+    return FileResponse(filepath)
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
async def update_board(request):
return FileResponse(f'{shared_path}/{request.path_params["board"]}/{request.path_params["filename"]}.bin')
async def update_board(request):
filepath = f'{shared_path}/{request.path_params["board"]}/{request.path_params["filename"]}.bin'
if not os.path.isfile(filepath):
raise HTTPException(status_code=404, detail="File not found")
return FileResponse(filepath)

firmware/src/JaamFirmware.cpp Show resolved Hide resolved
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

♻️ Duplicate comments (1)
deploy/websocket_server/websocket_server.py (1)

403-403: Виправте логування конфіденційної інформації!

Виявлено потенційний витік конфіденційної інформації через логування. Рекомендується замінити логування чутливих даних на безпечніший формат.

Also applies to: 410-410, 417-417, 424-424

🧹 Nitpick comments (1)
deploy/websocket_server/websocket_server.py (1)

870-889: Оптимізуйте код за допомогою тернарного оператора.

Замініть блоки if-else на тернарний оператор для кращої читабельності коду.

Застосуйте цей diff:

-    if s3_bins_cached:
-        s3_bins_cached_data = s3_bins_cached.decode("utf-8")
-    else:
-        s3_bins_cached_data = "[]"
+    s3_bins_cached_data = s3_bins_cached.decode("utf-8") if s3_bins_cached else "[]"

-    if s3_test_bins_cached:
-        s3_test_bins_cached_data = s3_test_bins_cached.decode("utf-8")
-    else:
-        s3_test_bins_cached_data = "[]"
+    s3_test_bins_cached_data = s3_test_bins_cached.decode("utf-8") if s3_test_bins_cached else "[]"

-    if c3_bins_cached:
-        c3_bins_cached_data = c3_bins_cached.decode("utf-8")
-    else:
-        c3_bins_cached_data = "[]"
+    c3_bins_cached_data = c3_bins_cached.decode("utf-8") if c3_bins_cached else "[]"

-    if c3_test_bins_cached:
-        c3_test_bins_cached_data = c3_test_bins_cached.decode("utf-8")
-    else:
-        c3_test_bins_cached_data = "[]"
+    c3_test_bins_cached_data = c3_test_bins_cached.decode("utf-8") if c3_test_bins_cached else "[]"
🧰 Tools
🪛 Ruff (0.8.2)

870-873: Use ternary operator s3_bins_cached_data = s3_bins_cached.decode("utf-8") if s3_bins_cached else "[]" instead of if-else-block

Replace if-else-block with s3_bins_cached_data = s3_bins_cached.decode("utf-8") if s3_bins_cached else "[]"

(SIM108)


880-883: Use ternary operator c3_bins_cached_data = c3_bins_cached.decode("utf-8") if c3_bins_cached else "[]" instead of if-else-block

Replace if-else-block with c3_bins_cached_data = c3_bins_cached.decode("utf-8") if c3_bins_cached else "[]"

(SIM108)

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between b31a790 and f607b55.

📒 Files selected for processing (1)
  • deploy/websocket_server/websocket_server.py (11 hunks)
🧰 Additional context used
🪛 Ruff (0.8.2)
deploy/websocket_server/websocket_server.py

870-873: Use ternary operator s3_bins_cached_data = s3_bins_cached.decode("utf-8") if s3_bins_cached else "[]" instead of if-else-block

Replace if-else-block with s3_bins_cached_data = s3_bins_cached.decode("utf-8") if s3_bins_cached else "[]"

(SIM108)


880-883: Use ternary operator c3_bins_cached_data = c3_bins_cached.decode("utf-8") if c3_bins_cached else "[]" instead of if-else-block

Replace if-else-block with c3_bins_cached_data = c3_bins_cached.decode("utf-8") if c3_bins_cached else "[]"

(SIM108)

🪛 GitHub Actions: 🧹 Lint Check
deploy/websocket_server/websocket_server.py

[error] 1-1: File would be reformatted. Please run Black to format the code.

⏰ Context from checks skipped due to timeout of 90000ms (4)
  • GitHub Check: compile_firmware_telnet
  • GitHub Check: compile_firmware_lite
  • GitHub Check: compile_firmware_esp32s3
  • GitHub Check: compile_firmware_esp32c3
🔇 Additional comments (4)
deploy/websocket_server/websocket_server.py (4)

80-83: LGTM! Додано нові атрибути для підтримки ESP32-S3 та ESP32-C3.

Правильно ініціалізовано нові атрибути для зберігання бінарних файлів для платформ ESP32-S3 та ESP32-C3.


374-425: LGTM! Коректна обробка прошивок для різних платформ.

Логіка правильно розділяє відправку бінарних файлів в залежності від типу прошивки (стандартна, S3 або C3).


677-704: LGTM! Коректна обробка оновлень для S3 та C3.

Правильно реалізовано оновлення бінарних файлів для нових платформ з відповідною обробкою помилок та логуванням.


797-800: LGTM! Додано отримання даних для S3 та C3 з memcached.

Правильно реалізовано отримання бінарних файлів для нових платформ з memcached.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

🧹 Nitpick comments (3)
deploy/websocket_server/websocket_server.py (3)

396-423: Рефакторинг: Зменшити дублювання коду.

Код для обробки бінів S3 та C3 майже ідентичний. Пропоную винести спільну логіку в окрему функцію.

+async def handle_board_bins(client, shared_bins, shared_test_bins, board_type, firmware, client_ip, chip_id, websocket):
+    if client["bins"] != shared_bins and f"-{board_type}" in firmware:
+        temp_bins = list(json.loads(shared_bins))
+        temp_bins.sort(key=bin_sort, reverse=True)
+        payload = '{"payload": "bins", "bins": %s}' % temp_bins
+        await websocket.send(payload)
+        logger.debug(f"{client_ip}:{chip_id} <<< new {board_type}_bins")
+        client["bins"] = shared_bins
+    
+    if client["test_bins"] != shared_test_bins and f"-{board_type}" in firmware:
+        temp_bins = list(json.loads(shared_test_bins))
+        temp_bins.sort(key=bin_sort, reverse=True)
+        payload = '{"payload": "test_bins", "test_bins": %s}' % temp_bins
+        await websocket.send(payload)
+        logger.debug(f"{client_ip}:{chip_id} <<< new {board_type}_test_bins")
+        client["test_bins"] = shared_test_bins

-            if client["bins"] != shared_data.s3_bins and "-s3" in firmware:
-                temp_bins = list(json.loads(shared_data.s3_bins))
-                temp_bins.sort(key=bin_sort, reverse=True)
-                payload = '{"payload": "bins", "bins": %s}' % temp_bins
-                await websocket.send(payload)
-                logger.debug(f"{client_ip}:{chip_id} <<< new s3_bins")
-                client["bins"] = shared_data.s3_bins
-            if client["test_bins"] != shared_data.s3_test_bins and "-s3" in firmware:
-                temp_bins = list(json.loads(shared_data.s3_test_bins))
-                temp_bins.sort(key=bin_sort, reverse=True)
-                payload = '{"payload": "test_bins", "test_bins": %s}' % temp_bins
-                await websocket.send(payload)
-                logger.debug(f"{client_ip}:{chip_id} <<< new s3_test_bins")
-                client["test_bins"] = shared_data.s3_test_bins
-            if client["bins"] != shared_data.c3_bins and "-c3" in firmware:
-                temp_bins = list(json.loads(shared_data.c3_bins))
-                temp_bins.sort(key=bin_sort, reverse=True)
-                payload = '{"payload": "bins", "bins": %s}' % temp_bins
-                await websocket.send(payload)
-                logger.debug(f"{client_ip}:{chip_id} <<< new c3_bins")
-                client["bins"] = shared_data.c3_bins
-            if client["test_bins"] != shared_data.c3_test_bins and "-c3" in firmware:
-                temp_bins = list(json.loads(shared_data.c3_test_bins))
-                temp_bins.sort(key=bin_sort, reverse=True)
-                payload = '{"payload": "test_bins", "test_bins": %s}' % temp_bins
-                await websocket.send(payload)
-                logger.debug(f"{client_ip}:{chip_id} <<< new c3_test_bins")
-                client["test_bins"] = shared_data.c3_test_bins
+            await handle_board_bins(client, shared_data.s3_bins, shared_data.s3_test_bins, "s3", firmware, client_ip, chip_id, websocket)
+            await handle_board_bins(client, shared_data.c3_bins, shared_data.c3_test_bins, "c3", firmware, client_ip, chip_id, websocket)

675-701: Рефакторинг: Зменшити дублювання коду в обробці помилок.

Код обробки помилок для бінів S3 та C3 дублюється. Пропоную створити допоміжну функцію.

+def update_board_bins(shared_data, board_type, bins, test_bins):
+    try:
+        bins_attr = f"{board_type}_bins"
+        test_bins_attr = f"{board_type}_test_bins"
+        if bins != getattr(shared_data, bins_attr):
+            setattr(shared_data, bins_attr, bins)
+            logger.debug(f"{board_type} bins updated: {bins}")
+    except Exception as e:
+        logger.error(f"error in {board_type} bins: {e}")
+
+    try:
+        if test_bins != getattr(shared_data, test_bins_attr):
+            setattr(shared_data, test_bins_attr, test_bins)
+            logger.debug(f"{board_type} test bins updated: {test_bins}")
+    except Exception as e:
+        logger.error(f"error in {board_type} test_bins: {e}")

-        try:
-            if s3_bins != shared_data.s3_bins:
-                shared_data.s3_bins = s3_bins
-                logger.debug(f"s3 bins updated: {s3_bins}")
-        except Exception as e:
-            logger.error(f"error in s3 bins: {e}")
-
-        try:
-            if s3_test_bins != shared_data.s3_test_bins:
-                shared_data.s3_test_bins = s3_test_bins
-                logger.debug(f"s3 test bins updated: {s3_test_bins}")
-        except Exception as e:
-            logger.error(f"error in s3 test_bins: {e}")
-
-        try:
-            if c3_bins != shared_data.c3_bins:
-                shared_data.c3_bins = c3_bins
-                logger.debug(f"c3 bins updated: {c3_bins}")
-        except Exception as e:
-            logger.error(f"error in c3 bins: {e}")
-
-        try:
-            if c3_test_bins != shared_data.c3_test_bins:
-                shared_data.c3_test_bins = c3_test_bins
-                logger.debug(f"c3 test bins updated: {c3_test_bins}")
-        except Exception as e:
-            logger.error(f"error in c3 test_bins: {e}")
+        update_board_bins(shared_data, "s3", s3_bins, s3_test_bins)
+        update_board_bins(shared_data, "c3", c3_bins, c3_test_bins)

868-887: Оптимізація: Використовувати тернарний оператор.

Код отримання даних з memcached можна зробити більш лаконічним.

-    if s3_bins_cached:
-        s3_bins_cached_data = s3_bins_cached.decode("utf-8")
-    else:
-        s3_bins_cached_data = "[]"
+    s3_bins_cached_data = s3_bins_cached.decode("utf-8") if s3_bins_cached else "[]"

-    if s3_test_bins_cached:
-        s3_test_bins_cached_data = s3_test_bins_cached.decode("utf-8")
-    else:
-        s3_test_bins_cached_data = "[]"
+    s3_test_bins_cached_data = s3_test_bins_cached.decode("utf-8") if s3_test_bins_cached else "[]"

-    if c3_bins_cached:
-        c3_bins_cached_data = c3_bins_cached.decode("utf-8")
-    else:
-        c3_bins_cached_data = "[]"
+    c3_bins_cached_data = c3_bins_cached.decode("utf-8") if c3_bins_cached else "[]"

-    if c3_test_bins_cached:
-        c3_test_bins_cached_data = c3_test_bins_cached.decode("utf-8")
-    else:
-        c3_test_bins_cached_data = "[]"
+    c3_test_bins_cached_data = c3_test_bins_cached.decode("utf-8") if c3_test_bins_cached else "[]"
🧰 Tools
🪛 Ruff (0.8.2)

868-871: Use ternary operator s3_bins_cached_data = s3_bins_cached.decode("utf-8") if s3_bins_cached else "[]" instead of if-else-block

Replace if-else-block with s3_bins_cached_data = s3_bins_cached.decode("utf-8") if s3_bins_cached else "[]"

(SIM108)


878-881: Use ternary operator c3_bins_cached_data = c3_bins_cached.decode("utf-8") if c3_bins_cached else "[]" instead of if-else-block

Replace if-else-block with c3_bins_cached_data = c3_bins_cached.decode("utf-8") if c3_bins_cached else "[]"

(SIM108)

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between f607b55 and 732c254.

📒 Files selected for processing (1)
  • deploy/websocket_server/websocket_server.py (11 hunks)
🧰 Additional context used
🪛 Ruff (0.8.2)
deploy/websocket_server/websocket_server.py

868-871: Use ternary operator s3_bins_cached_data = s3_bins_cached.decode("utf-8") if s3_bins_cached else "[]" instead of if-else-block

Replace if-else-block with s3_bins_cached_data = s3_bins_cached.decode("utf-8") if s3_bins_cached else "[]"

(SIM108)


878-881: Use ternary operator c3_bins_cached_data = c3_bins_cached.decode("utf-8") if c3_bins_cached else "[]" instead of if-else-block

Replace if-else-block with c3_bins_cached_data = c3_bins_cached.decode("utf-8") if c3_bins_cached else "[]"

(SIM108)

⏰ Context from checks skipped due to timeout of 90000ms (5)
  • GitHub Check: compile_firmware_telnet
  • GitHub Check: compile_firmware_lite
  • GitHub Check: compile_firmware_esp32s3
  • GitHub Check: compile_firmware_esp32c3
  • GitHub Check: compile_firmware
🔇 Additional comments (2)
deploy/websocket_server/websocket_server.py (2)

80-83: LGTM: Нові атрибути для ESP32-S3 та ESP32-C3.

Додані атрибути дотримуються існуючого шаблону та правильно ініціалізуються порожніми JSON масивами.


376-377: LGTM: Виправлено перевірку типу прошивки.

Правильно використано оператор in замість неіснуючого методу contains().

Also applies to: 386-387

@Foroxon Foroxon changed the title Нова сумісність: ESP32-S3, ESP32-C3, Нова сумісність: ESP32-S3, ESP32-C3 Feb 10, 2025
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

♻️ Duplicate comments (1)
firmware/src/Constants.h (1)

351-353: Додати перевірку валідності піна в макрос.

Макрос GENERATE_PIN_CASE потребує вбудованої перевірки валідності піна.

Оновіть макрос для включення валідації:

-#define GENERATE_PIN_CASE(pin) \
-    case pin: FastLED.addLeds<NEOPIXEL, pin>(const_cast<CRGB*>(leds), pixelcount); break;
+#define GENERATE_PIN_CASE(pin) \
+    case pin: \
+        validatePin(pin); \
+        FastLED.addLeds<NEOPIXEL, pin>(const_cast<CRGB*>(leds), pixelcount); \
+        break;

Committable suggestion skipped: line range outside the PR's diff.

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 583ee27 and 51f15e5.

📒 Files selected for processing (2)
  • firmware/src/Constants.h (1 hunks)
  • firmware/src/JaamFirmware.cpp (4 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • firmware/src/JaamFirmware.cpp
⏰ Context from checks skipped due to timeout of 90000ms (6)
  • GitHub Check: compile_jaam2_test
  • GitHub Check: compile_firmware_telnet
  • GitHub Check: compile_firmware_lite
  • GitHub Check: compile_firmware_esp32s3
  • GitHub Check: compile_firmware_esp32c3
  • GitHub Check: compile_firmware
🔇 Additional comments (1)
firmware/src/Constants.h (1)

324-336: LGTM! Коректна обробка платформо-залежних опцій.

Правильне використання препроцесорних директив для визначення кількості legacy опцій в залежності від платформи.

Comment on lines 340 to 349
// Визначення пінів для різних плат
#if ARDUINO_ESP32_DEV
#define SUPPORTED_LEDS_PINS {2, 4, 12, 13, 14, 15, 16, 17, 18, 25, 26, 27, 32, 33}
#elif ARDUINO_ESP32S3_DEV
#define SUPPORTED_LEDS_PINS {2, 4, 12, 13, 14, 15, 18, 21, 25, 26, 33, 34, 35 ,36, 37, 38, 39, 40, 41, 42}
#elif ARDUINO_ESP32C3_DEV
#define SUPPORTED_LEDS_PINS {2 ,3 ,4 ,5 ,6 ,7 ,8 ,9 ,10 ,18 ,19 ,20 ,21}
#else
#error "Платформа не підтримується!"
#endif
Copy link
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Додати валідацію пінів для кожної платформи.

Поточна реалізація визначає підтримувані піни, але не має механізму валідації під час виконання.

Пропоную додати функцію для перевірки валідності пінів:

// Функція для перевірки підтримки піна на поточній платформі
constexpr bool isPinSupported(const int pin) {
    constexpr int supported_pins[] = SUPPORTED_LEDS_PINS;
    for (const auto supported_pin : supported_pins) {
        if (pin == supported_pin) return true;
    }
    return false;
}

// Функція для валідації піна з викиданням помилки
void validatePin(const int pin) {
    if (!isPinSupported(pin)) {
        throw std::invalid_argument("LED пін не підтримується на цій платформі");
    }
}

# Conflicts:
#	firmware/src/JaamFirmware.cpp
@v00g100skr v00g100skr merged commit 76b1639 into develop Feb 10, 2025
14 checks passed
@v00g100skr v00g100skr deleted the esp32s3 branch February 10, 2025 20:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
покращення прошивки This will not be worked on
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[П] Підтримка ESP32-C3
2 participants