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

Fixes Particle.connect() behavior in SEMI_AUTOMATIC mode #1403

Closed
wants to merge 3 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 13 additions & 0 deletions system/src/main.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -407,17 +407,26 @@ void manage_safe_mode()
}
}

bool semi_automatic_connecting(bool threaded) {
return system_mode() == SEMI_AUTOMATIC && !threaded && spark_cloud_flag_auto_connect() && !spark_cloud_flag_connected();
}

void app_loop(bool threaded)
{
DECLARE_SYS_HEALTH(ENTERED_WLAN_Loop);
if (!threaded)
Spark_Idle();

static uint8_t SPARK_WIRING_APPLICATION = 0;
do {
Copy link
Contributor

Choose a reason for hiding this comment

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

what's the purpose of the do/while(false) - isn't it the same as a regular block?

Copy link
Member Author

Choose a reason for hiding this comment

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

We need to short-circuit the execution at line https://github.com/spark/firmware/pull/1403/files#diff-17aa9c74822820227ce1b5db3aab8d76R527 We can change that to goto if it looks better :)

if(threaded || SPARK_WLAN_SLEEP || !spark_cloud_flag_auto_connect() || spark_cloud_flag_connected() || SPARK_WIRING_APPLICATION || (system_mode()!=AUTOMATIC))
{
if(threaded || !SPARK_FLASH_UPDATE)
{
if (semi_automatic_connecting(threaded)) {
Copy link
Contributor

Choose a reason for hiding this comment

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

The diff isn't so clear in github, but logically, I imagine this is the primary change - to not invoke loop while connecting in semi-automatic mode.

Copy link
Member Author

@avtolstoy avtolstoy Nov 21, 2017

Choose a reason for hiding this comment

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

break;
}

if ((SPARK_WIRING_APPLICATION != 1))
{
//Execute user application setup only once
Expand All @@ -428,6 +437,9 @@ void app_loop(bool threaded)
#if !(defined(MODULAR_FIRMWARE) && MODULAR_FIRMWARE)
_post_loop();
#endif
if (semi_automatic_connecting(threaded)) {
break;
}
}

//Execute user application loop
Expand All @@ -441,6 +453,7 @@ void app_loop(bool threaded)
}
}
}
} while(false);
#if PLATFORM_ID == 3 && SUSPEND_APPLICATION_THREAD_LOOP_COUNT
// Suspend thread execution for some minimum time on every Nth loop iteration in order to workaround
// 100% CPU usage on the virtual device platform
Expand Down
84 changes: 84 additions & 0 deletions user/tests/app/particle_connect_semi_automatic_issue_1399/test.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,84 @@
#include "application.h"

SYSTEM_MODE(SEMI_AUTOMATIC);
SYSTEM_THREAD(DISABLED);

LOG_SOURCE_CATEGORY("test")

namespace {

SerialLogHandler logHandler(LOG_LEVEL_NONE, {
{ "test", LOG_LEVEL_ALL }
});

static bool loopExecuted = false;
static bool testFinished = false;

static void fail() {
testFinished = true;
RGB.control(true);
RGB.color(0xff0000); // Red
LOG(ERROR, "TEST FAILED");
}

static void pass() {
testFinished = true;
RGB.control(true);
RGB.color(0x00ff00); // Green
LOG(INFO, "TEST SUCCEEDED");
}

} // namespace

void setup() {
waitUntil(Serial.isConnected);

LOG(INFO, "Test started");
LOG(INFO, "Connecting to WiFi network");
WiFi.on();
WiFi.connect();
waitUntil(WiFi.ready);
LOG(INFO, "Connected to WiFi network");

LOG(INFO, "Connecting to the cloud");
Particle.connect();
if (Particle.connected()) {
fail();
}
}

void loop() {
if (testFinished) {
return;
}

if (!loopExecuted) {
loopExecuted = true;

// First time the loop is running. We should be connected to the cloud
if (!Particle.connected()) {
fail();
return;
}

LOG(INFO, "Connected to the cloud");

// Disconnect from the cloud
LOG(INFO, "Disconnecting from the cloud");
Particle.disconnect();
waitUntil(Particle.disconnected);
LOG(INFO, "Disconnected from the cloud");

LOG(INFO, "Connecting to the cloud");
Particle.connect();
} else {
// Second time the loop is running, we should also be connected to the cloud again
if (!Particle.connected()) {
fail();
return;
}
LOG(INFO, "Connected to the cloud");

pass();
}
}
25 changes: 1 addition & 24 deletions user/tests/wiring/no_fixture/cloud.cpp
Original file line number Diff line number Diff line change
@@ -1,37 +1,14 @@
#include "application.h"
#include "unit-test/unit-test.h"

test(CLOUD_01_Particle_Connect_Blocks_In_SemiAutomatic_Mode_With_Threading_Disabled) {
if (system_thread_get_state(nullptr) == spark::feature::ENABLED) {
skip();
return;
}

Particle.disconnect();
waitFor(Particle.disconnected, 10000);
assertTrue(Particle.disconnected());

// Switch to SEMI_AUTOMATIC mode
set_system_mode(SEMI_AUTOMATIC);

Particle.connect();
assertTrue(Particle.connected());
}

test(CLOUD_02_Particle_Connect_Does_Not_Block_In_SemiAutomatic_Mode_With_Threading_Enabled) {
if (system_thread_get_state(nullptr) == spark::feature::DISABLED) {
skip();
return;
}
test(CLOUD_01_Particle_Connect_Does_Not_Block_In_SemiAutomatic_Mode) {
Particle.disconnect();
waitFor(Particle.disconnected, 10000);
assertTrue(Particle.disconnected());

// Switch to SEMI_AUTOMATIC mode
set_system_mode(SEMI_AUTOMATIC);

assertTrue(system_thread_get_state(nullptr) == spark::feature::ENABLED);

Particle.connect();
assertFalse(Particle.connected());

Expand Down
6 changes: 0 additions & 6 deletions wiring/inc/spark_wiring_cloud.h
Original file line number Diff line number Diff line change
Expand Up @@ -325,12 +325,6 @@ class CloudClass {
static bool disconnected(void) { return !connected(); }
static void connect(void) {
spark_cloud_flag_connect();
if (system_thread_get_state(nullptr)==spark::feature::DISABLED &&
SystemClass::mode() == SEMI_AUTOMATIC)
{
// Particle.connect() should be blocking in SEMI_AUTOMATIC mode when threading is disabled
waitUntil(connected);
}
}
static void disconnect(void) { spark_cloud_flag_disconnect(); }
static void process(void) {
Expand Down