Skip to content

Commit

Permalink
Clean up migration to ESP logging.
Browse files Browse the repository at this point in the history
Whilst here, noticed we're leaking client connections, so migrate to
unique_ptr to handle it.
  • Loading branch information
gkoh committed Sep 1, 2024
1 parent 39bdcbf commit 49e7534
Show file tree
Hide file tree
Showing 9 changed files with 63 additions and 49 deletions.
10 changes: 9 additions & 1 deletion lib/furble/Camera.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4,13 +4,21 @@

namespace Furble {

Camera::Camera() {
m_Client = NimBLEDevice::createClient();
}

Camera::~Camera() {
NimBLEDevice::deleteClient(m_Client);
}

bool Camera::connect(esp_power_level_t power, progressFunc pFunc, void *pCtx) {
// try extending range by adjusting connection parameters
m_Client->updateConnParams(m_MinInterval, m_MaxInterval, m_Latency, m_Timeout);
bool connected = this->connect(pFunc, pCtx);
if (connected) {
// Set BLE transmit power after connection is established.
NimBLEDevice::setPower(power);
m_Client->updateConnParams(m_MinInterval, m_MaxInterval, m_Latency, m_Timeout);
}

return connected;
Expand Down
7 changes: 5 additions & 2 deletions lib/furble/Camera.h
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,9 @@ namespace Furble {
*/
class Camera {
public:
Camera();
~Camera();

/**
* GPS data type.
*/
Expand Down Expand Up @@ -99,8 +102,8 @@ class Camera {
*/
virtual bool connect(progressFunc pFunc = nullptr, void *pCtx = nullptr) = 0;

NimBLEAddress m_Address = NimBLEAddress("");
NimBLEClient *m_Client = NimBLEDevice::createClient();
NimBLEAddress m_Address = NimBLEAddress{};
NimBLEClient *m_Client;
std::string m_Name;

void updateProgress(progressFunc pFunc, void *ctx, float value);
Expand Down
36 changes: 18 additions & 18 deletions lib/furble/CameraList.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@

namespace Furble {

std::vector<Furble::Camera *> CameraList::m_ConnectList;
std::vector<std::unique_ptr<Furble::Camera>> CameraList::m_ConnectList;
static Preferences m_Prefs;

/**
Expand Down Expand Up @@ -72,19 +72,19 @@ static void add_index(std::vector<index_entry_t> &index, index_entry_t &entry) {
}
}

void CameraList::save(Camera *pCamera) {
void CameraList::save(Furble::Camera *camera) {
m_Prefs.begin(FURBLE_STR, false);
std::vector<index_entry_t> index = load_index();

index_entry_t entry = {0};
pCamera->fillSaveName(entry.name);
entry.type = pCamera->getDeviceType();
camera->fillSaveName(entry.name);
entry.type = camera->getDeviceType();

add_index(index, entry);

size_t dbytes = pCamera->getSerialisedBytes();
size_t dbytes = camera->getSerialisedBytes();
uint8_t dbuffer[dbytes] = {0};
if (pCamera->serialise(dbuffer, dbytes)) {
if (camera->serialise(dbuffer, dbytes)) {
// Store the entry and the index if serialisation succeeds
m_Prefs.putBytes(entry.name, dbuffer, dbytes);
ESP_LOGI(LOG_TAG, "Saved %s\r\n", entry.name);
Expand All @@ -95,12 +95,12 @@ void CameraList::save(Camera *pCamera) {
m_Prefs.end();
}

void CameraList::remove(Camera *pCamera) {
void CameraList::remove(Furble::Camera *camera) {
m_Prefs.begin(FURBLE_STR, false);
std::vector<index_entry_t> index = load_index();

index_entry_t entry = {0};
pCamera->fillSaveName(entry.name);
camera->fillSaveName(entry.name);

size_t i = 0;
for (i = 0; i < index.size(); i++) {
Expand Down Expand Up @@ -140,16 +140,16 @@ void CameraList::load(void) {

switch (index[i].type) {
case FURBLE_FUJIFILM:
m_ConnectList.push_back(new Fujifilm(dbuffer, dbytes));
m_ConnectList.push_back(std::unique_ptr<Furble::Camera>(new Fujifilm(dbuffer, dbytes)));
break;
case FURBLE_CANON_EOS_M6:
m_ConnectList.push_back(new CanonEOSM6(dbuffer, dbytes));
m_ConnectList.push_back(std::unique_ptr<Furble::Camera>(new CanonEOSM6(dbuffer, dbytes)));
break;
case FURBLE_CANON_EOS_RP:
m_ConnectList.push_back(new CanonEOSRP(dbuffer, dbytes));
m_ConnectList.push_back(std::unique_ptr<Furble::Camera>(new CanonEOSRP(dbuffer, dbytes)));
break;
case FURBLE_MOBILE_DEVICE:
m_ConnectList.push_back(new MobileDevice(dbuffer, dbytes));
m_ConnectList.push_back(std::unique_ptr<Furble::Camera>(new MobileDevice(dbuffer, dbytes)));
break;
}
}
Expand All @@ -173,30 +173,30 @@ void CameraList::clear(void) {
}

Furble::Camera *CameraList::get(size_t n) {
return m_ConnectList[n];
return m_ConnectList[n].get();
}

Furble::Camera *CameraList::back(void) {
return m_ConnectList.back();
return m_ConnectList.back().get();
}

bool CameraList::match(NimBLEAdvertisedDevice *pDevice) {
if (Fujifilm::matches(pDevice)) {
m_ConnectList.push_back(new Furble::Fujifilm(pDevice));
m_ConnectList.push_back(std::unique_ptr<Furble::Camera>(new Furble::Fujifilm(pDevice)));
return true;
} else if (CanonEOSM6::matches(pDevice)) {
m_ConnectList.push_back(new Furble::CanonEOSM6(pDevice));
m_ConnectList.push_back(std::unique_ptr<Furble::Camera>(new Furble::CanonEOSM6(pDevice)));
return true;
} else if (CanonEOSRP::matches(pDevice)) {
m_ConnectList.push_back(new Furble::CanonEOSRP(pDevice));
m_ConnectList.push_back(std::unique_ptr<Furble::Camera>(new Furble::CanonEOSRP(pDevice)));
return true;
}

return false;
}

void CameraList::add(NimBLEAddress address) {
m_ConnectList.push_back(new Furble::MobileDevice(address));
m_ConnectList.push_back(std::unique_ptr<Furble::Camera>(new Furble::MobileDevice(address)));
}

} // namespace Furble
10 changes: 6 additions & 4 deletions lib/furble/CameraList.h
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
#ifndef CAMERALIST_H
#define CAMERALIST_H

#include <memory>

#include "Camera.h"

namespace Furble {
Expand All @@ -12,12 +14,12 @@ class CameraList {
/**
* Save camera to connection list.
*/
static void save(Camera *pCamera);
static void save(Furble::Camera *camera);

/**
* Remove camera from connection list.
*/
static void remove(Camera *pCamera);
static void remove(Furble::Camera *camera);

/**
* Load previously connected devices.
Expand Down Expand Up @@ -59,13 +61,13 @@ class CameraList {
/**
* Retrieve last entry.
*/
static Camera *back(void);
static Furble::Camera *back(void);

private:
/**
* List of connectable devices.
*/
static std::vector<Furble::Camera *> m_ConnectList;
static std::vector<std::unique_ptr<Furble::Camera>> m_ConnectList;
};
} // namespace Furble

Expand Down
2 changes: 1 addition & 1 deletion lib/furble/CanonEOS.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -160,7 +160,7 @@ bool CanonEOS::connect(progressFunc pFunc, void *pCtx) {

if (m_PairResult != CANON_EOS_PAIR_ACCEPT) {
bool deleted = NimBLEDevice::deleteBond(m_Address);
ESP_LOGI(LOG_TAG, "Rejected, delete pairing: %d", deleted);
ESP_LOGW(LOG_TAG, "Rejected, delete pairing: %d", deleted);
return false;
}

Expand Down
38 changes: 19 additions & 19 deletions lib/furble/Fujifilm.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -61,11 +61,11 @@ static void print_token(const std::array<uint8_t, FUJIFILM_TOKEN_LEN> &token) {
}

void Fujifilm::notify(BLERemoteCharacteristic *pChr, uint8_t *pData, size_t length, bool isNotify) {
ESP_LOGW(LOG_TAG, "Got %s callback: %u bytes from %s", isNotify ? "notification" : "indication",
ESP_LOGI(LOG_TAG, "Got %s callback: %u bytes from %s", isNotify ? "notification" : "indication",
length, pChr->getUUID().toString().c_str());
if (length > 0) {
for (int i = 0; i < length; i++) {
ESP_LOGW(LOG_TAG, " [%d] 0x%02x", i, pData[i]);
ESP_LOGI(LOG_TAG, " [%d] 0x%02x", i, pData[i]);
}
}

Expand Down Expand Up @@ -97,8 +97,8 @@ Fujifilm::Fujifilm(NimBLEAdvertisedDevice *pDevice) {
m_Name = pDevice->getName();
m_Address = pDevice->getAddress();
m_Token = {data[3], data[4], data[5], data[6]};
ESP_LOGW(LOG_TAG, "Name = %s", m_Name.c_str());
ESP_LOGW(LOG_TAG, "Address = %s", m_Address.toString().c_str());
ESP_LOGI(LOG_TAG, "Name = %s", m_Name.c_str());
ESP_LOGI(LOG_TAG, "Address = %s", m_Address.toString().c_str());
print_token(m_Token);
}

Expand Down Expand Up @@ -141,17 +141,17 @@ bool Fujifilm::connect(progressFunc pFunc, void *pCtx) {
NimBLERemoteService *pSvc = nullptr;
NimBLERemoteCharacteristic *pChr = nullptr;

ESP_LOGW(LOG_TAG, "Connecting");
ESP_LOGI(LOG_TAG, "Connecting");
if (!m_Client->connect(m_Address))
return false;

ESP_LOGW(LOG_TAG, "Connected");
ESP_LOGI(LOG_TAG, "Connected");
updateProgress(pFunc, pCtx, 20.0f);
pSvc = m_Client->getService(FUJIFILM_SVC_PAIR_UUID);
if (pSvc == nullptr)
return false;

ESP_LOGW(LOG_TAG, "Pairing");
ESP_LOGI(LOG_TAG, "Pairing");
pChr = pSvc->getCharacteristic(FUJIFILM_CHR_PAIR_UUID);
if (pChr == nullptr)
return false;
Expand All @@ -161,19 +161,19 @@ bool Fujifilm::connect(progressFunc pFunc, void *pCtx) {
print_token(m_Token);
if (!pChr->writeValue(m_Token.data(), sizeof(m_Token), true))
return false;
ESP_LOGW(LOG_TAG, "Paired!");
ESP_LOGI(LOG_TAG, "Paired!");
updateProgress(pFunc, pCtx, 30.0f);

ESP_LOGW(LOG_TAG, "Identifying");
ESP_LOGI(LOG_TAG, "Identifying");
pChr = pSvc->getCharacteristic(FUJIFILM_CHR_IDEN_UUID);
if (!pChr->canWrite())
return false;
if (!pChr->writeValue(Device::getStringID(), true))
return false;
ESP_LOGW(LOG_TAG, "Identified!");
ESP_LOGI(LOG_TAG, "Identified!");
updateProgress(pFunc, pCtx, 40.0f);

ESP_LOGW(LOG_TAG, "Configuring");
ESP_LOGI(LOG_TAG, "Configuring");
pSvc = m_Client->getService(FUJIFILM_SVC_CONF_UUID);
// indications
pSvc->getCharacteristic(FUJIFILM_CHR_IND1_UUID)
Expand Down Expand Up @@ -229,7 +229,7 @@ bool Fujifilm::connect(progressFunc pFunc, void *pCtx) {
this->notify(pChr, pData, length, isNotify);
},
true);
ESP_LOGW(LOG_TAG, "Configured");
ESP_LOGI(LOG_TAG, "Configured");

updateProgress(pFunc, pCtx, 100.0f);

Expand Down Expand Up @@ -286,11 +286,11 @@ void Fujifilm::sendGeoData(gps_t &gps, timesync_t &timesync) {
.second = (uint8_t)timesync.second,
}};

ESP_LOGW(LOG_TAG, "Sending geotag data (%u bytes) to 0x%04x", sizeof(geotag),
ESP_LOGI(LOG_TAG, "Sending geotag data (%u bytes) to 0x%04x", sizeof(geotag),
pChr->getHandle());
ESP_LOGW(LOG_TAG, " lat: %f, %d", gps.latitude, geotag.latitude);
ESP_LOGW(LOG_TAG, " lon: %f, %d", gps.longitude, geotag.longitude);
ESP_LOGW(LOG_TAG, " alt: %f, %d", gps.altitude, geotag.altitude);
ESP_LOGI(LOG_TAG, " lat: %f, %d", gps.latitude, geotag.latitude);
ESP_LOGI(LOG_TAG, " lon: %f, %d", gps.longitude, geotag.longitude);
ESP_LOGI(LOG_TAG, " alt: %f, %d", gps.altitude, geotag.altitude);

pChr->writeValue((uint8_t *)&geotag, sizeof(geotag), true);
}
Expand All @@ -304,9 +304,9 @@ void Fujifilm::updateGeoData(gps_t &gps, timesync_t &timesync) {
}

void Fujifilm::print(void) {
ESP_LOGW(LOG_TAG, "Name: %s", m_Name.c_str());
ESP_LOGW(LOG_TAG, "Address: %s", m_Address.toString().c_str());
ESP_LOGW(LOG_TAG, "Type: %d", m_Address.getType());
ESP_LOGI(LOG_TAG, "Name: %s", m_Name.c_str());
ESP_LOGI(LOG_TAG, "Address: %s", m_Address.toString().c_str());
ESP_LOGI(LOG_TAG, "Type: %d", m_Address.getType());
}

void Fujifilm::disconnect(void) {
Expand Down
3 changes: 2 additions & 1 deletion platformio.ini
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
[furble]
build_flags = -Wall -DFURBLE_VERSION=\"${sysenv.FURBLE_VERSION}\" -DCORE_DEBUG_LEVEL=2
build_flags = -Wall -DFURBLE_VERSION=\"${sysenv.FURBLE_VERSION}\" -DCORE_DEBUG_LEVEL=3

[env]
platform = espressif32
Expand All @@ -14,6 +14,7 @@ lib_deps =

[env:m5stick-c]
board = m5stick-c
build_flags = ${furble.build_flags}

[env:m5stack-core]
board = m5stack-core-esp32
Expand Down
4 changes: 2 additions & 2 deletions src/furble.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ static void show_shutter_control(bool shutter_locked, unsigned long lock_start_m
}

static void remote_control(FurbleCtx *fctx) {
Furble::Camera *camera = fctx->camera;
auto camera = fctx->camera;
static unsigned long shutter_lock_start_ms = 0;
static bool shutter_lock = false;

Expand Down Expand Up @@ -150,7 +150,7 @@ static void remote_control(FurbleCtx *fctx) {
*/
static uint16_t statusRefresh(void *private_data) {
FurbleCtx *fctx = static_cast<FurbleCtx *>(private_data);
Furble::Camera *camera = fctx->camera;
auto camera = fctx->camera;

if (camera->isConnected()) {
furble_gps_update_geodata(camera);
Expand Down
2 changes: 1 addition & 1 deletion src/interval.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ static void display_interval_msg(interval_state_t state,
}

static void do_interval(FurbleCtx *fctx, interval_t *interval) {
Furble::Camera *camera = fctx->camera;
auto camera = fctx->camera;
const unsigned long config_delay = sv2ms(&interval->delay);
const unsigned long config_shutter = sv2ms(&interval->shutter);

Expand Down

0 comments on commit 49e7534

Please sign in to comment.