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

Prevent OTA progress events from blocking system thread #2741

Merged
merged 6 commits into from
Feb 26, 2024

Conversation

sergeuz
Copy link
Member

@sergeuz sergeuz commented Feb 19, 2024

Problem

Emitting a firmware update progress event blocks the system thread if the event queue of the application thread is full.

Solution

Introduce a flag for system_notify_event() that allows the system to drop the event if the event queue is full.

This PR also removes the unused event queue implementation based on the cpp::channel library. Interestingly, that seems to have caused a problem when linking the wiring library with a user app that uses Print::print(double) in my local environment even though it's not reproducible on the CI. As a workaround, I inlined methods that get conditionally compiled based on the PARTICLE_WIRING_PRINT_NO_FLOAT macro since we only build the wiring library once and can't have different compile time options for it depending on where it's used.

Steps to Test

  1. Flash the test application to a device OTA.
  2. When the device restarts, flash some large enough firmware to the device OTA.
  3. Validate that flashing finishes in a reasonable time.

Test Application

#include "application.h"

SYSTEM_MODE(SEMI_AUTOMATIC)
SYSTEM_THREAD(ENABLED)

namespace {

void onFirmwareUpdateEvent(system_event_t event, int param, void* data) {
    if (param == firmware_update_progress) {
        HAL_Delay_Milliseconds(3000);
    }
}

} // namespace

void setup() {
    System.on(firmware_update, onFirmwareUpdateEvent);
    Particle.connect();
}

void loop() {
}

References

  • [sc-125421]

@sergeuz sergeuz force-pushed the drop_ota_chunk_events/sc-125421 branch 3 times, most recently from 0a18618 to e6ffca3 Compare February 19, 2024 20:59
@mrlambchop
Copy link
Contributor

Folks - whilst I appreciate the work gone into fix this OTA issue, I recognize this an old troubled friend in the world of event queues and message passing and would like to discuss this at the system design level before it lands. Will pop into a standup to discuss!

@sergeuz sergeuz force-pushed the drop_ota_chunk_events/sc-125421 branch from e6ffca3 to c34b320 Compare February 22, 2024 11:49
@sergeuz sergeuz force-pushed the drop_ota_chunk_events/sc-125421 branch from c34b320 to 570aab7 Compare February 22, 2024 14:27
@sergeuz sergeuz marked this pull request as ready for review February 26, 2024 12:29
@sergeuz sergeuz force-pushed the drop_ota_chunk_events/sc-125421 branch from 9475159 to 28e9d84 Compare February 26, 2024 12:38
@sergeuz sergeuz merged commit 4436513 into develop Feb 26, 2024
13 checks passed
@sergeuz sergeuz deleted the drop_ota_chunk_events/sc-125421 branch February 26, 2024 13:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants