From 120d6cc3ff52e236d142d5247dcefff822e1c88e Mon Sep 17 00:00:00 2001 From: "michael.toon" Date: Thu, 15 Aug 2019 11:51:14 +0100 Subject: [PATCH] BLOCKS: Add master block detection and throttle ping requests This adds master block detection and will update the topology if necessary. This fixes an issue where master block could be wrongly identified. Added a throttle to ping requests by staggering them between timer ticks, this mitigates some issues with larger topologies. --- .../internal/juce_BlockImplementation.cpp | 29 ++-- .../internal/juce_BlockSerialReader.cpp | 130 ++++++++++++++++++ .../internal/juce_ConnectedDeviceGroup.cpp | 120 +++++++++++++--- .../topology/internal/juce_Detector.cpp | 40 ++++-- .../topology/internal/juce_DetectorHolder.cpp | 17 ++- .../internal/juce_MidiDeviceConnection.cpp | 2 +- .../topology/juce_PhysicalTopologySource.cpp | 21 ++- 7 files changed, 309 insertions(+), 50 deletions(-) create mode 100644 modules/juce_blocks_basics/topology/internal/juce_BlockSerialReader.cpp diff --git a/modules/juce_blocks_basics/topology/internal/juce_BlockImplementation.cpp b/modules/juce_blocks_basics/topology/internal/juce_BlockImplementation.cpp index f7f581c1ce..9bc3dcf124 100644 --- a/modules/juce_blocks_basics/topology/internal/juce_BlockImplementation.cpp +++ b/modules/juce_blocks_basics/topology/internal/juce_BlockImplementation.cpp @@ -83,13 +83,7 @@ public: if (connectionTime == Time()) connectionTime = Time::getCurrentTime(); - versionNumber = deviceInfo.version.asString(); - name = deviceInfo.name.asString(); - isMaster = deviceInfo.isMaster; - masterUID = deviceInfo.masterUid; - batteryCharging = deviceInfo.batteryCharging; - batteryLevel = deviceInfo.batteryLevel; - topologyIndex = deviceInfo.index; + updateDeviceInfo (deviceInfo); setProgram (nullptr); remoteHeap.resetDeviceStateToUnknown(); @@ -100,6 +94,17 @@ public: updateMidiConnectionListener(); } + void updateDeviceInfo (const DeviceInfo& deviceInfo) + { + versionNumber = deviceInfo.version.asString(); + name = deviceInfo.name.asString(); + isMaster = deviceInfo.isMaster; + masterUID = deviceInfo.masterUid; + batteryCharging = deviceInfo.batteryCharging; + batteryLevel = deviceInfo.batteryLevel; + topologyIndex = deviceInfo.index; + } + void setToMaster (bool shouldBeMaster) { isMaster = shouldBeMaster; @@ -504,10 +509,15 @@ public: remoteHeap.sendChanges (*this, false); - if (lastMessageSendTime < Time::getCurrentTime() - RelativeTime::milliseconds (pingIntervalMs)) + if (lastMessageSendTime < Time::getCurrentTime() - getPingInterval()) sendCommandMessage (BlocksProtocol::ping); } + RelativeTime getPingInterval() + { + return RelativeTime::milliseconds (isMaster ? masterPingIntervalMs : dnaPingIntervalMs); + } + //============================================================================== int32 getLocalConfigValue (uint32 item) override { @@ -629,7 +639,8 @@ public: MIDIDeviceConnection* listenerToMidiConnection = nullptr; - static constexpr int pingIntervalMs = 400; + static constexpr int masterPingIntervalMs = 400; + static constexpr int dnaPingIntervalMs = 1666; static constexpr uint32 maxBlockSize = BlocksProtocol::padBlockProgramAndHeapSize; static constexpr uint32 maxPacketCounter = BlocksProtocol::PacketCounter::maxValue; diff --git a/modules/juce_blocks_basics/topology/internal/juce_BlockSerialReader.cpp b/modules/juce_blocks_basics/topology/internal/juce_BlockSerialReader.cpp new file mode 100644 index 0000000000..c4a94e9276 --- /dev/null +++ b/modules/juce_blocks_basics/topology/internal/juce_BlockSerialReader.cpp @@ -0,0 +1,130 @@ +/* + ============================================================================== + + This file is part of the JUCE library. + Copyright (c) 2019 - ROLI Ltd. + + JUCE is an open source library subject to commercial or open-source + licensing. + + The code included in this file is provided under the terms of the ISC license + http://www.isc.org/downloads/software-support-policy/isc-license. Permission + To use, copy, modify, and/or distribute this software for any purpose with or + without fee is hereby granted provided that the above copyright notice and + this permission notice appear in all copies. + + JUCE IS PROVIDED "AS IS" WITHOUT ANY WARRANTY, AND ALL WARRANTIES, WHETHER + EXPRESSED OR IMPLIED, INCLUDING MERCHANTABILITY AND FITNESS FOR PURPOSE, ARE + DISCLAIMED. + + ============================================================================== +*/ + +namespace juce +{ + class BlockSerialReader + : private MIDIDeviceConnection::Listener, + private Timer + { + public: + //============================================================================== + BlockSerialReader (MIDIDeviceConnection& deviceConnectionToUse) : deviceConnection (deviceConnectionToUse) + { + deviceConnection.addListener (this); + startTimer (10); + } + + ~BlockSerialReader() override + { + deviceConnection.removeListener (this); + } + + bool hasSerial() const { return serial.isNotEmpty(); } + + String getSerial() const { return serial; } + + private: + MIDIDeviceConnection& deviceConnection; + String serial; + + bool shouldStop() { return hasSerial(); } + + //============================================================================== + void timerCallback() override + { + if (shouldStop()) + { + stopTimer(); + return; + } + + sendRequest(); + startTimer (300); + } + + void sendRequest() + { + const uint8 dumpRequest[] = { 0xf0, 0x00, 0x21, 0x10, 0x78, 0x3f, 0xf7 }; + deviceConnection.sendMessageToDevice (dumpRequest, sizeof (dumpRequest)); + } + + void handleIncomingMidiMessage (const MidiMessage& message) override + { + if (hasSerial()) + return; + + if (isResponse (message)) + parseResponse (message); + } + + void connectionBeingDeleted (const MIDIDeviceConnection&) override + { + stopTimer(); + } + + bool isResponse (const MidiMessage message) + { + const uint8 roliDumpHeader[] = { 0xf0, 0x00, 0x21, 0x10, 0x78}; + return memcmp (message.getRawData(), roliDumpHeader, sizeof (roliDumpHeader)) == 0; + } + + void parseResponse (const MidiMessage& message) + { + int index = findMacAddressStart (message); + + if (index >= 0) + { + const int macSize = 17; + const int offset = index + macSize; + const int serialSize = 16; + + if (message.getRawDataSize() - offset < serialSize) + { + jassertfalse; + return; + } + + serial = String ((const char*)message.getRawData() + offset, serialSize); + } + } + + int findMacAddressStart (const MidiMessage& message) + { + const uint8 macStart[] = { '4', '8', ':', 'B', '6', ':', '2', '0', ':' }; + return findSequence (macStart, sizeof (macStart), message); + } + + int findSequence (const uint8* sequence, int sequenceSize, const MidiMessage& message) + { + for (int i = 0; i < message.getRawDataSize() - sequenceSize; i++) + { + if (memcmp (message.getRawData() + i, sequence, size_t (sequenceSize)) == 0) + return i; + } + + return -1; + } + + JUCE_DECLARE_NON_COPYABLE_WITH_LEAK_DETECTOR (BlockSerialReader) + }; +} diff --git a/modules/juce_blocks_basics/topology/internal/juce_ConnectedDeviceGroup.cpp b/modules/juce_blocks_basics/topology/internal/juce_ConnectedDeviceGroup.cpp index cab84e8850..5d4c8615fb 100644 --- a/modules/juce_blocks_basics/topology/internal/juce_ConnectedDeviceGroup.cpp +++ b/modules/juce_blocks_basics/topology/internal/juce_ConnectedDeviceGroup.cpp @@ -49,6 +49,8 @@ struct ConnectedDeviceGroup : private AsyncUpdater, setMidiMessageCallback(); } + initialiseSerialReader(); + startTimer (200); sendTopologyRequest(); } @@ -294,12 +296,13 @@ private: Array incomingPackets; std::unique_ptr depreciatedVersionReader; + std::unique_ptr masterSerialReader; struct TouchStart { float x, y; }; TouchList touchStartPositions; static constexpr Block::UID invalidUid = 0; - Block::UID masterBlock = invalidUid; + Block::UID masterBlockUid = invalidUid; //============================================================================== void timerCallback() override @@ -313,7 +316,8 @@ private: checkApiTimeouts (now); startApiModeOnConnectedBlocks(); - requestMasterBlockVersionIfNeeded(); + checkMasterBlockVersion(); + checkMasterSerial(); } //============================================================================== @@ -394,7 +398,7 @@ private: } //============================================================================== - void requestMasterBlockVersionIfNeeded() + void checkMasterBlockVersion() { if (depreciatedVersionReader == nullptr) return; @@ -403,7 +407,7 @@ private: if (masterVersion.isNotEmpty()) { - const auto masterIndex = getIndexFromDeviceID (masterBlock); + const auto masterIndex = getIndexFromDeviceID (masterBlockUid); if (masterIndex >= 0) setVersion (BlocksProtocol::TopologyIndex (masterIndex), masterVersion); @@ -422,7 +426,7 @@ private: if (info->version == versionNumber) return; - if (info->uid == masterBlock) + if (info->uid == masterBlockUid) depreciatedVersionReader.reset(); info->version = versionNumber; @@ -430,6 +434,72 @@ private: } } + //============================================================================== + void checkMasterSerial() + { + if (masterSerialReader == nullptr) + initialiseSerialReader(); + + if (masterSerialReader == nullptr) + return; + + if (masterBlockUid != invalidUid && masterSerialReader->hasSerial()) + { + auto uid = getBlockUIDFromSerialNumber (masterSerialReader->getSerial()); + + if (uid != masterBlockUid) + updateMasterUid (uid); + } + } + + void updateMasterUid (const Block::UID newMasterUid) + { + LOG_CONNECTIVITY ("Updating master from " + String (masterBlockUid) + " to " + String (newMasterUid)); + + masterBlockUid = newMasterUid; + + Array devicesToUpdate; + + for (auto& info : currentDeviceInfo) + { + if (info.masterUid != masterBlockUid) + { + info.masterUid = masterBlockUid; + + info.isMaster = info.uid == masterBlockUid; + + devicesToUpdate.add (info); + } + } + + detector.handleDevicesUpdated (devicesToUpdate); + } + + Block::UID determineMasterBlockUid (Array devices) + { + if (masterSerialReader != nullptr && masterSerialReader->hasSerial()) + { + auto foundSerial = masterSerialReader->getSerial(); + for (const auto& device : incomingTopologyDevices) + { + if (device.serialNumber.asString() == foundSerial) + { + LOG_CONNECTIVITY ("Found master from serial " + foundSerial); + return getBlockUIDFromSerialNumber (foundSerial); + } + } + } + + if (devices.size() > 0) + { + LOG_CONNECTIVITY ("Found master from first device " + devices[0].serialNumber.asString()); + return getBlockUIDFromSerialNumber (incomingTopologyDevices[0].serialNumber); + } + + jassertfalse; + return invalidUid; + } + //============================================================================== struct BlockPingTime { @@ -491,15 +561,16 @@ private: void forceApiDisconnected (Block::UID uid) { - Array toRemove; - for (auto dependentUID : detector.getDnaDependentDeviceUIDs (uid)) removeDevice (dependentUID); removeDevice (uid); - if (uid == masterBlock) - masterBlock = invalidUid; + if (uid == masterBlockUid) + { + masterBlockUid = invalidUid; + masterSerialReader.reset(); + } scheduleNewTopologyRequest(); } @@ -637,22 +708,19 @@ private: for (const auto& uid : toRemove) removeDevice (uid); + if (masterBlockUid == invalidUid) + { + masterBlockUid = determineMasterBlockUid (incomingTopologyDevices); + initialiseVersionReader(); + } + //Add new devices for (const auto& device : incomingTopologyDevices) { const auto uid = getBlockUIDFromSerialNumber (device.serialNumber); - if (! getDeviceInfoFromUID (uid)) + if (getDeviceInfoFromUID (uid) == nullptr) { - // For backwards compatibility we assume the first device we see in a group is the master and won't change - if (masterBlock == invalidUid) - { - masterBlock = uid; - - if (auto midiDeviceConnection = static_cast (deviceConnection.get())) - depreciatedVersionReader = std::make_unique (*midiDeviceConnection); - } - currentDeviceInfo.add ({ uid, device.index, device.serialNumber, @@ -660,7 +728,7 @@ private: BlocksProtocol::BlockName(), device.batteryLevel, device.batteryCharging, - masterBlock }); + masterBlockUid }); } } } @@ -699,6 +767,18 @@ private: detector.handleConnectionsChanged(); } + void initialiseVersionReader() + { + if (auto midiDeviceConnection = static_cast (deviceConnection.get())) + depreciatedVersionReader = std::make_unique (*midiDeviceConnection); + } + + void initialiseSerialReader() + { + if (auto midiDeviceConnection = static_cast (deviceConnection.get())) + masterSerialReader = std::make_unique (*midiDeviceConnection); + } + JUCE_DECLARE_NON_COPYABLE_WITH_LEAK_DETECTOR (ConnectedDeviceGroup) }; diff --git a/modules/juce_blocks_basics/topology/internal/juce_Detector.cpp b/modules/juce_blocks_basics/topology/internal/juce_Detector.cpp index 1fb6c6924d..09bffab625 100644 --- a/modules/juce_blocks_basics/topology/internal/juce_Detector.cpp +++ b/modules/juce_blocks_basics/topology/internal/juce_Detector.cpp @@ -177,27 +177,41 @@ struct Detector : public ReferenceCountedObject, triggerAsyncUpdate(); } - void handleDeviceUpdated (const DeviceInfo& info) + void handleDevicesUpdated (const Array& infos) { - if (containsBlockWithUID (blocksToRemove, info.uid)) - return; + bool shouldTriggerUpdate { false }; - const auto blockIt = std::find_if (currentTopology.blocks.begin(), currentTopology.blocks.end(), - [uid = info.uid] (Block::Ptr block) { return uid == block->uid; }); - - if (blockIt != currentTopology.blocks.end()) + for (auto& info : infos) { - const Block::Ptr block { *blockIt }; + if (containsBlockWithUID (blocksToRemove, info.uid)) + continue; - if (auto blockImpl = BlockImpl::getFrom (block.get())) - blockImpl->markReconnected (info); + const auto blockIt = std::find_if (currentTopology.blocks.begin(), currentTopology.blocks.end(), + [uid = info.uid] (Block::Ptr block) { return uid == block->uid; }); - if (! containsBlockWithUID (blocksToAdd, info.uid)) + + if (blockIt != currentTopology.blocks.end()) { - blocksToUpdate.addIfNotAlreadyThere (block); - triggerAsyncUpdate(); + const Block::Ptr block { *blockIt }; + + if (auto blockImpl = BlockImpl::getFrom (block.get())) + blockImpl->updateDeviceInfo (info); + + if (! containsBlockWithUID (blocksToAdd, info.uid)) + { + blocksToUpdate.addIfNotAlreadyThere (block); + shouldTriggerUpdate = true; + } } } + + if (shouldTriggerUpdate) + triggerAsyncUpdate(); + } + + void handleDeviceUpdated (const DeviceInfo& info) + { + handleDevicesUpdated ({ info }); } void handleBatteryChargingChanged (Block::UID deviceID, const BlocksProtocol::BatteryCharging isCharging) diff --git a/modules/juce_blocks_basics/topology/internal/juce_DetectorHolder.cpp b/modules/juce_blocks_basics/topology/internal/juce_DetectorHolder.cpp index b2297cef04..eab5583f18 100644 --- a/modules/juce_blocks_basics/topology/internal/juce_DetectorHolder.cpp +++ b/modules/juce_blocks_basics/topology/internal/juce_DetectorHolder.cpp @@ -47,13 +47,24 @@ struct PhysicalTopologySource::DetectorHolder : private Timer void handleTimerTick() { - for (auto& b : detector->currentTopology.blocks) - if (auto bi = BlockImplementation::getFrom (*b)) - bi->handleTimerTick(); + auto blocks = detector->currentTopology.blocks; + + if (blocks.size() == 0) + return; + + if (nextIndexToTick >= blocks.size()) + nextIndexToTick = 0; + + if (auto* bi = BlockImplementation::getFrom (*blocks [nextIndexToTick])) + bi->handleTimerTick(); + + nextIndexToTick = (nextIndexToTick + 1) % blocks.size(); } PhysicalTopologySource& topologySource; Detector::Ptr detector; + + int nextIndexToTick {0}; }; } // namespace juce diff --git a/modules/juce_blocks_basics/topology/internal/juce_MidiDeviceConnection.cpp b/modules/juce_blocks_basics/topology/internal/juce_MidiDeviceConnection.cpp index 56fb9e34c1..6861db6693 100644 --- a/modules/juce_blocks_basics/topology/internal/juce_MidiDeviceConnection.cpp +++ b/modules/juce_blocks_basics/topology/internal/juce_MidiDeviceConnection.cpp @@ -67,7 +67,7 @@ struct MIDIDeviceConnection : public PhysicalTopologySource::DeviceConnection, { JUCE_ASSERT_MESSAGE_MANAGER_IS_LOCKED // This method must only be called from the message thread! - jassert (dataSize > sizeof (BlocksProtocol::roliSysexHeader) + 2); + jassert (dataSize > sizeof (BlocksProtocol::roliSysexHeader) + 1); jassert (memcmp (data, BlocksProtocol::roliSysexHeader, sizeof (BlocksProtocol::roliSysexHeader) - 1) == 0); jassert (static_cast (data)[dataSize - 1] == 0xf7); diff --git a/modules/juce_blocks_basics/topology/juce_PhysicalTopologySource.cpp b/modules/juce_blocks_basics/topology/juce_PhysicalTopologySource.cpp index 521473e048..e08d54b8b4 100644 --- a/modules/juce_blocks_basics/topology/juce_PhysicalTopologySource.cpp +++ b/modules/juce_blocks_basics/topology/juce_PhysicalTopologySource.cpp @@ -22,10 +22,22 @@ //============================================================================== /** These can be useful when debugging the topology. */ -#define LOG_BLOCKS_CONNECTIVITY 0 -#define LOG_BLOCKS_PINGS 0 -#define DUMP_BANDWIDTH_STATS 0 -#define DUMP_TOPOLOGY 0 + +#ifndef LOG_BLOCKS_CONNECTIVITY + #define LOG_BLOCKS_CONNECTIVITY 0 +#endif + +#ifndef LOG_BLOCKS_PINGS + #define LOG_BLOCKS_PINGS 0 +#endif + +#ifndef DUMP_BANDWIDTH_STATS + #define DUMP_BANDWIDTH_STATS 0 +#endif + +#ifndef DUMP_TOPOLOGY + #define DUMP_TOPOLOGY 0 +#endif #define TOPOLOGY_LOG(text) \ JUCE_BLOCK_WITH_FORCED_SEMICOLON (juce::String buf ("Topology Src: "); \ @@ -51,6 +63,7 @@ #include "internal/juce_MIDIDeviceDetector.cpp" #include "internal/juce_DeviceInfo.cpp" #include "internal/juce_DepreciatedVersionReader.cpp" +#include "internal/juce_BlockSerialReader.cpp" #include "internal/juce_ConnectedDeviceGroup.cpp" #include "internal/juce_BlockImplementation.cpp" #include "internal/juce_Detector.cpp"