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

Fix error handling in the data usage API (Electron) #1435

Merged
merged 1 commit into from Dec 1, 2017

Conversation

@sergeuz
Copy link
Member

commented Nov 17, 2017

Problem

This PR makes Cellular::getDataUsage() and Cellular::setDataUsage() return false if the data usage counters cannot be retrieved or updated (e.g. if the network is turned off).

Note: #1171 reports a similar problem with Cellular::resetDataUsage(), but I wasn't able to reproduce it.

Example App

Below is the test application that can be used to check the data usage under various scenarios:

#include "application.h"

SYSTEM_MODE(MANUAL)

namespace {

const Serial1LogHandler logHandler(115200, LOG_LEVEL_WARN, {
    { "app", LOG_LEVEL_ALL }
});

CellularData dataUsage;

void logDataUsage() {
    Log.info("ok: %s; cid: %d; tx_session: %d; rx_session: %d; tx_total: %d; rx_total: %d",
            (dataUsage.ok ? "true" : "false"), dataUsage.cid, dataUsage.tx_session, dataUsage.rx_session,
            dataUsage.tx_total, dataUsage.rx_total);
}

void getDataUsage() {
    Log.info("Getting data usage");
    if (!Cellular.getDataUsage(dataUsage)) {
        Log.error("Cellular.getDataUsage() failed");
    }
    logDataUsage();
}

void setDataUsage() {
    Log.info("Setting data usage");
    if (!Cellular.setDataUsage(dataUsage)) {
        Log.error("Cellular.setDataUsage() failed");
    }
    logDataUsage();
}

void resetDataUsage() {
    Log.info("Resetting data usage");
    if (Cellular.resetDataUsage()) {
        getDataUsage();
    } else {
        Log.error("Cellular.resetDataUsage() failed");
    }
}

void off() {
    Log.info("Turning network off");
    Cellular.off();
    Log.info("Network is off");
    delay(1000);
    getDataUsage();
}

void connect() {
    Log.info("Connecting to cloud");
    Particle.connect();
    waitUntil(Particle.connected);
    Log.info("Connected to cloud");
    delay(1000);
    getDataUsage();
}

void publish() {
    Log.info("Publishing event");
    bool done = false;
    Particle.publish("test", PRIVATE | WITH_ACK).onSuccess([&done](bool) {
        Log.info("Received ACK");
        done = true;
    }).onError([&done](Error) {
        done = true;
    });
    while (!done) {
        Particle.process();
    }
    delay(1000);
    getDataUsage();
}

void test() {
    Log.print("======== Basic test\r\n");
    off();
    connect();
    publish();
    off();
    connect();
    publish();

    Log.print("======== Setting data usage\r\n");
    dataUsage.tx_session += 100000;
    dataUsage.rx_session += 100000;
    dataUsage.tx_total += 100000;
    dataUsage.rx_total += 100000;
    setDataUsage();
    publish();

    Log.print("======== Resetting data usage\r\n");
    resetDataUsage();
    publish();

    Log.print("======== Setting data usage when network is off\r\n");
    off();
    setDataUsage();

    Log.print("======== Resetting data usage when network is off\r\n");
    resetDataUsage();
}

} // namespace

void setup() {
    test();
}

void loop() {
}

References

  • [CH9469]

Completeness

  • User is totes amazing for contributing!
  • Contributor has signed CLA (Info here)
  • Problem and Solution clearly stated
  • Run unit/integration/application tests on device
  • Added documentation
  • Added to CHANGELOG.md after merging (add links to docs and issues)

@sergeuz sergeuz added the bug label Nov 17, 2017

@sergeuz sergeuz added this to the 0.8.0 milestone Nov 17, 2017

@sergeuz sergeuz requested a review from technobly Nov 17, 2017

@technobly
Copy link
Member

left a comment

Looks good. Did you run the test code in #1171 ? What were your observations there?

@sergeuz sergeuz force-pushed the fix/issue_1171 branch from b4964fc to 21e5194 Dec 1, 2017

@sergeuz

This comment has been minimized.

Copy link
Member Author

commented Dec 1, 2017

Rebased on develop

@sergeuz

This comment has been minimized.

Copy link
Member Author

commented Dec 1, 2017

Did you run the test code in #1171 ? What were your observations there?

Cellular::resetDataUsage() seems to handle errors correctly: https://github.com/spark/firmware/blob/fix/issue_1171/wiring/src/spark_wiring_cellular.cpp#L87, and I wasn't able to reproduce the original issue

@technobly

This comment has been minimized.

Copy link
Member

commented Dec 1, 2017

I just looked closer at the code, and it appears as if the modem is off, it should report that it errored. Perhaps I had logging disabled :)

@m-mcgowan m-mcgowan modified the milestones: 0.8.0, 0.8.0-rc.1 Dec 1, 2017

@m-mcgowan m-mcgowan merged commit 3618b65 into develop Dec 1, 2017

2 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.