1
\$\begingroup\$

I have some C++ based firmware for the Particle Photon that I've recently refactored which could do with a review, first time writing C++.

Any thoughts are welcome, including design and style (I tried to mostly follow the Google style guide for C++).

For context, the firmware manages three relays that can be used to unlock a door if the handset is being called. There are also a few input lines, like door status (open/closed), and the call tone line (a square wave when active).

You can prime the system so that it automatically unlocks the door if the handset is called within a time limit, or just unlock it whenever the handset is being called.

Here's the documentation for the firmware: https://docs.particle.io/reference/firmware/photon/

Here's the gatekeeper.ino file:

// This #include statement was automatically added by the Particle IDE.
#include "Gatekeeper.h"

// Create an instance of the Gatekeeper library.
Gatekeeper gatekeeper;

// The variables we will publish to the Particle service.
int is_door_open_;
int is_handset_active_;
int is_primed_;

void setup() {
    gatekeeper.Begin();

    Particle.variable("isActive", is_handset_active_);
    Particle.variable("isDoorOpen", is_door_open_);
    Particle.variable("isPrimed", is_primed_);
}

void check(int &variable, bool (Gatekeeper::*function)(), String triggerEventName, String detriggerEventName) {
    bool cachedValue = (gatekeeper.*function)();

    if (! variable && cachedValue) {
        variable = 1;
        Particle.publish(String("gatekeeper/" + triggerEventName), PRIVATE);
    } else if (variable && ! cachedValue) {
        variable = 0;
        Particle.publish(String("gatekeeper/" + detriggerEventName), PRIVATE);
    }
}

void loop() {
    check(is_primed_, &Gatekeeper::IsPrimed, "primed", "unprimed");
    check(is_door_open_, &Gatekeeper::IsDoorOpen, "door-opened", "door-closed");
    check(is_handset_active_, &Gatekeeper::IsHandsetActive, "handset-activated", "handset-deactivated");

    // Unlock the door if the handset is active and primed.
    if (is_handset_active_ && is_primed_) {
        gatekeeper.Unlock();
    }
}

Here's Gatekeeper.h:

#ifndef Gatekeeper_H_
#define Gatekeeper_H_

#include "application.h"

//
// A library for interacting with the Gatekeeper system.
//
class Gatekeeper
{
    public:

        Gatekeeper();

        void Begin();
        bool Unlock();
        bool Prime();

        bool IsDoorOpen();
        bool IsHandsetActive();
        bool IsPrimed();

        void CallToneInteruptHandler();

    private:

        int Unlock(String command);
        int Prime(String command);
};

#endif // Gatekeeper_H_

Lastly, here's Gatekeeper.cpp:

#include "Gatekeeper.h"

const int kPinDrRelay = D0;
const int kPinS1Relay = D1;
const int kPinS2Relay = D2;
const int kPinDsLine = D3;
const int kPinBzLine = D4;

int primed_;

volatile int last_bz_interrupt_;

Gatekeeper::Gatekeeper() {}

void Gatekeeper::Begin() {
    // Register Particle functions.
    Particle.function("unlock", &Gatekeeper::Unlock, this);
    Particle.function("prime", &Gatekeeper::Prime, this);

    // Setup the relay signals.
    pinMode(kPinDrRelay, OUTPUT);
    pinMode(kPinS1Relay, OUTPUT);
    pinMode(kPinS2Relay, OUTPUT);

    // Setup the BS line to read the door status.
    pinMode(kPinDsLine, INPUT_PULLUP);

    // Setup the BZ line to read the call tone with an interrupt.
    pinMode(kPinBzLine, INPUT_PULLDOWN);
    attachInterrupt(kPinBzLine, &Gatekeeper::CallToneInteruptHandler, this, RISING);

    // Initialise the output pins.
    digitalWrite(kPinDrRelay, LOW);
    digitalWrite(kPinS1Relay, LOW);
    digitalWrite(kPinS2Relay, LOW);

    Particle.publish("gatekeeper/setup", PRIVATE);
}

bool Gatekeeper::Unlock() {
    // If the handset is not active, or the door's already open we can't unlock it.
    if (! IsHandsetActive() || IsDoorOpen()) {
        return false;
    }

    // "Pickup" the handset and "press" the door release button.
    digitalWrite(kPinS1Relay, HIGH);
    digitalWrite(kPinS2Relay, HIGH);

    delay(300);

    digitalWrite(kPinDrRelay, HIGH);

    delay(200);

    // Finally resetting all the relays.
    digitalWrite(kPinDrRelay, LOW);
    digitalWrite(kPinS1Relay, LOW);
    digitalWrite(kPinS2Relay, LOW);

    // Ensure we unprime the system.
    primed_ = 0;

    Particle.publish("gatekeeper/unlocked", PRIVATE);

    return true;
}

bool Gatekeeper::Prime() {
    // If the door's open, what's the point in priming?
    if (IsDoorOpen()) {
        return false;
    }

    primed_ = Time.now();

    return true;
}

bool Gatekeeper::IsDoorOpen() {
    return (digitalRead(kPinDsLine) == LOW);
}

bool Gatekeeper::IsHandsetActive() {
    return (last_bz_interrupt_ > Time.now() - 1 && pulseIn(kPinBzLine, HIGH) > 0);
}

bool Gatekeeper::IsPrimed() {
    return (primed_ > Time.now() - 120);
}

void Gatekeeper::CallToneInteruptHandler() {
    last_bz_interrupt_ = Time.now();
}

int Gatekeeper::Unlock(String command) {
    return Unlock() ? 0 : -1;
}

int Gatekeeper::Prime(String command) {
    return Prime() ? 0 : -1;
}
\$\endgroup\$
2
  • \$\begingroup\$ I tried to mostly follow the Google style Though the google c++ style guide has improved a lot over time it is not consider a good general purpose C++ style guide. It is designed specifically to handle C++ code in google which has some subtle historical roots. This book is witten by two of the biggest nmaes in C++ and is probably a much better fit. \$\endgroup\$ Apr 18 '16 at 9:45
  • \$\begingroup\$ Ah very interesting @LokiAstari, makes sense when you know! I'll have a look at that book. \$\endgroup\$ Apr 18 '16 at 10:51

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Browse other questions tagged or ask your own question.