BLE streamlining #13365

paul-szczepanek-arm · 2020-07-28T17:22:22Z

Summary of changes

This is a refactor of BLE that makes Cordio the official BLE stack. There are no changes from the user's perspective. The API remains the same. This is to lessen the maintenance load. Other implementations are welcome but must implement the user API directly.

The internal changes are as follows:

removed the generic layer which is now the direct implementation of the interface
removed templated implementations
removed nested namespaces in favour of unique names (unambiguous, do not rely on namespace context)
removed support for multiple instances within the BLE singleton (which was already the case, see below)

This retains the separation at the PAL level, but without the templated implementation. Alternative implementations are possible by implementing the PAL API.

In simple words:
BLE is now a singleton. All implementation are non virtual concrete classes that implement interfaces directly. Any alternative implementation will have to be switched at compile time (see mbed_lib.json).

Impact of changes

Migration actions required

There are no other known BLE implementations but any porter who has one will need to adjust their implementation of BLEInstanceBase as we no longer support multiple instances within the BLE singleton. Multiple instances of BLE are supported but must be managed by the user/porter by creating their own instance of BLE.

This is already the case. The change is that this is now enforced by the API.

Documentation

None

Pull request type

[x] Patch update (Bug fix / Target update / Docs update / Test update / Refactor)
[] Feature update (New feature / Functionality change / New API)
[] Major update (Breaking change E.g. Return code change / API behaviour change)

Test results

[] No Tests required for this change (E.g docs only update)
[] Covered by existing mbed-os tests (Greentea or Unittest)
[x] Tests / results supplied as part of this PR

Reviewers

@donatieng
@LDong-Arm

paul-szczepanek-arm · 2020-07-28T17:23:13Z

This has been tested with the new pytest suite.

ciarmcom · 2020-07-28T19:00:33Z

@paul-szczepanek-arm, thank you for your changes.
@donatieng @LDong-Arm @ARMmbed/mbed-os-maintainers please review.

LDong-Arm

Thanks @paul-szczepanek-arm, the structure looks good to me. Please see my comment regarding the inclusion of TARGET_CORDIO headers in generic code.

connectivity/FEATURE_BLE/cordio/TARGET_CORDIO/driver/CordioHCIDriver.h

LDong-Arm · 2020-07-31T11:06:38Z

connectivity/FEATURE_BLE/include/ble/internal/GapImpl.h

+
+class PalGenericAccessService;
+class PalSecurityManager;
+class PalGap;
+class BLEInstanceBase;


Should we include their headers instead? Same for other class <CLASS_NAME>; in other files.

can't, cyclic dependency

LDong-Arm · 2020-07-31T11:19:43Z

connectivity/FEATURE_BLE/include/ble/SecurityManager.h

 } // ble

-using ble::impl::SecurityManager;


For compatibility we should add Gap, SecurityManager, etc. back to the global namespace (with using for example). Our examples and cliapp should continue to compile.

I thought Gap and the rest are supposed to be in ble:: namespace, looking at the old files I don't see a using ble::Gap

Examples don't compile because they were incorrectly failing to include headers and relying on the happy coincidence of other headers including what they needed, I have made the fixes but not pushed to master yet

I added it as a config option

LDong-Arm · 2020-07-31T13:12:09Z

connectivity/FEATURE_BLE/include/ble/internal/PalGap.h

 } // namespace ble

+
+#if (BLE_PAL_API_IMPLEMENTATION == 1)
+#include "ble/internal/PalGapImpl.h"


This file is in TARGET_CORDIO - we'd better not include it here.
How to achieve that? In non-Cordio code, just use ble::interface::PalGap where possible, and this is the point of virtual functions/interface classes. It's only a few lines' change and it worked as far as I tried.
Same for the other virtual Pal classes.

Once the generic code has no dependency on headers in TARGET_CORDIO, the headers/classes in TARGET_CORDIO can be renamed to contain "Cordio" in my opinion.

The point is to remove virtual calls. Alternative implementation can override this include with their implementation.

paul-szczepanek-arm · 2020-08-04T12:06:01Z

@0xc0170 How do I make exceptions for astyle? this is thirdparty code that is tripping the astyle tests.

LDong-Arm · 2020-08-04T12:28:15Z

@0xc0170 How do I make exceptions for astyle? this is thirdparty code that is tripping the astyle tests.

Use .astyleignore located at the root of mbed-os.

mergify · 2020-08-04T12:46:03Z

This PR cannot be merged due to conflicts. Please rebase to resolve them.

paul-szczepanek-arm · 2020-08-04T12:49:01Z

Thank Lingkai, anything else I should look at?

LDong-Arm

The structure looks very good to me. I didn't and couldn't review every single line of diff given how big it is. But since the PR mostly cuts and pastes code around, we can assume nothing is missing as ble-tests and examples continue to work (otherwise things probably wouldn't even compile...).

LDong-Arm · 2020-08-04T14:44:43Z

.astyleignore

@@ -1,9 +1,9 @@
 ^BUILD
 ^cmsis
+^connectivity/FEATURE_BLE


Not related to this PR - no need to fix it here:
We'd better only ignore external components (e.g. Cordio) while keeping checks on our code. If any styles don't match, the CI generates a diff we can apply without manual fixes.

the old check was for features/FEATURE_BLE so I figured I'd better move it as is since I don't want to get bogged down with astyle right now, we can fix this in a later PR

LDong-Arm · 2020-08-04T15:10:01Z

connectivity/FEATURE_BLE/include/ble/Gap.h

+#if (BLE_API_IMPLEMENTATION == 1)
+#include "ble/internal/GapImpl.h"
+#else
+#error "please provide alternate BLE implementation"
+#endif


Suggested change

#if (BLE_API_IMPLEMENTATION == 1)

#include "ble/internal/GapImpl.h"

#else

#error "please provide alternate BLE implementation"

#endif

#include "ble/internal/GapImpl.h"

As discussed offline, this include is okay. I guess the standardised header name can be considered part of the "porting interface", and I've seen a few other projects doing this too.

Actually the BLE_API_IMPLEMENTATION macro may not be needed. If a user doesn't want Cordio, they can simply undeclare Cordio from the Mbed config so TARGET_CORDIO isn't built, and provide their implementation with the same "ble/internal/GapImpl.h" file path.

yes, totally agreed, this was only to show the porting point but this can be made better in a readme somewhere

LDong-Arm · 2020-08-04T15:13:00Z

connectivity/FEATURE_BLE/mbed_lib.json

+        "ble-api-implementation": {
+            "help": "What BLE user API implementation is used. Available options: 1 (Mbed). You may add more. See headers in include/ble/.",
+            "value": 1,
+            "macro_name": "BLE_API_IMPLEMENTATION"
+        },
+        "ble-pal-api-implementation": {
+            "help": "What Pal API implementation is used. Available options: 1 (Cordio). You may add more. See headers in include/ble/internal/Pal*.",
+            "value": 1,
+            "macro_name": "BLE_PAL_API_IMPLEMENTATION"
+        },


See my comment above. If impl header names are standardised, we can always assume they exist.
The TARGET_CORDIO magic name already does the switch to use Cordio (i.e. default) impl or not.

yep, let me remove that

LDong-Arm

@paul-szczepanek-arm Nitpicking over the public headers mostly. I'll approve this once these are resolved.

connectivity/FEATURE_BLE/include/ble/BLE.h

connectivity/FEATURE_BLE/include/ble/Gap.h

LDong-Arm · 2020-08-05T15:52:07Z

connectivity/FEATURE_BLE/include/ble/GattServer.h

+
+/*! Maximum count of characteristics that can be stored for authorisation purposes */
+#define MAX_CHARACTERISTIC_AUTHORIZATION_CNT 20
+
+/*! client characteristic configuration descriptors settings */
+#define MAX_CCCD_CNT 20


Duplicated from the Impl header which is a better place for those?

Suggested change

/*! Maximum count of characteristics that can be stored for authorisation purposes */

#define MAX_CHARACTERISTIC_AUTHORIZATION_CNT 20

/*! client characteristic configuration descriptors settings */

#define MAX_CCCD_CNT 20

they should not exist as defines at all, they should be a config - added

connectivity/FEATURE_BLE/include/ble/GattServer.h

LDong-Arm · 2020-08-05T15:57:20Z

connectivity/FEATURE_BLE/include/ble/GattServer.h

+    Gap::PreferredConnectionParams_t getPreferredConnectionParams();

    /**
-     * The registered callback handler for confirmation received events.
+     * Set preferred connection parameters.
+     *
+     * @param[in] params Preferred connection parameter values to set.
     */
-    EventCallback_t confirmationReceivedCallback;
-
-private:
-    /* Disallow copy and assignment. */
-    GattServer(const GattServer &);
-    GattServer& operator=(const GattServer &);
+    void setPreferredConnectionParams(const Gap::PreferredConnectionParams_t& params);


Thanks for adding them back - I removed them from the public API a few months (thought they were deprecated but were actually missing...), please mention them in the PR description as a new feature.

LDong-Arm · 2020-08-05T15:59:42Z

connectivity/FEATURE_BLE/include/ble/SecurityManager.h

+    /** For backwards compatibility. This enm is now in BLETypes.h
+     * @deprecated, use the enum in ble namespace */
+    enum Keypress_t {
+        KEYPRESS_STARTED,   /**< Passkey entry started */
+        KEYPRESS_ENTERED,   /**< Passkey digit entered */
+        KEYPRESS_ERASED,    /**< Passkey digit erased */
+        KEYPRESS_CLEARED,   /**< Passkey cleared */
+        KEYPRESS_COMPLETED, /**< Passkey entry completed */
+    };


Suggested change

/** For backwards compatibility. This enm is now in BLETypes.h

* @deprecated, use the enum in ble namespace */

enum Keypress_t {

KEYPRESS_STARTED, /**< Passkey entry started */

KEYPRESS_ENTERED, /**< Passkey digit entered */

KEYPRESS_ERASED, /**< Passkey digit erased */

KEYPRESS_CLEARED, /**< Passkey cleared */

KEYPRESS_COMPLETED, /**< Passkey entry completed */

};

/** For backwards compatibility. This enum is now in BLETypes.h

* @deprecated, use the enum in ble namespace */

using using ble::Keypress_t;

I assume you meant a typedef

connectivity/FEATURE_BLE/include/ble/internal/BLEInstanceBase.h

LDong-Arm

LGTM, thanks for it! @paul-szczepanek-arm

@adbridge Could you please trigger a CI run? Thanks.
I'll also try a few things locally tomorrow, though this kind of refactoring (moving files and classes around) should be low risk, as long as things compile.

adbridge · 2020-08-06T09:04:14Z

CI started

mbed-ci · 2020-08-06T09:55:48Z

Jenkins CI Test : ❌ FAILED

Build Number: 1 | 🔒 Jenkins CI Job | 🌐 Logs & Artifacts

CLICK for Detailed Summary

jobs	Status
jenkins-ci/mbed-os-ci_unittests	✔️
jenkins-ci/mbed-os-ci_build-GCC_ARM	❌
jenkins-ci/mbed-os-ci_build-ARM	❌

adbridge · 2020-08-06T10:40:58Z

@paul-szczepanek-arm looks like the failures are BLE related. For one of the failing boards I see:

[Error] pretty_printer.h@84,26: member access into incomplete type 'ble::Gap'
[Error] main.cpp@30,25: incomplete type 'ble::Gap' named in nested name specifier
[Error] main.cpp@30,30: expected class name
[Error] main.cpp@208,45: no type named 'DisconnectionCompleteEvent' in namespace 'ble'
[Error] main.cpp@217,10: no type named 'AdvertisingDataBuilder' in namespace 'ble'
[Error] main.cpp@38,19: member access into incomplete type 'ble::Gap'
[Error] main.cpp@147,35: expected ';' after expression
[Error] main.cpp@147,14: no member named 'AdvertisingParameters' in namespace 'ble'
[Error] main.cpp@147,36: use of undeclared identifier 'adv_parameters'
[Error] main.cpp@175,39: member access into incomplete type 'ble::Gap'
[Error] main.cpp@185,27: member access into incomplete type 'ble::Gap'
[Error] main.cpp@197,27: member access into incomplete type 'ble::Gap'
[Error] main.cpp@209,19: member access into incomplete type 'ble::Gap'
[ERROR] In file included from ./source/main.cpp:20:

LDong-Arm

Still looking into it...

LDong-Arm · 2020-08-06T10:32:44Z

connectivity/drivers/ble/COMPONENT_CYW43XXX/CyH4TransportDriver.h

@@ -21,7 +21,6 @@
 #if (DEVICE_SERIAL && DEVICE_SERIAL_FC) || defined(DOXYGEN_ONLY)

 #include <stdint.h>
-#include "mbed.h"


This is one of several causes of the CI failures.
Because this is removed, you need to

Include a number of headers (e.g. "drivers/InterruptIn.h")

Use the qualified class names (e.g. mbed::InterruptIn) instead of short one (e.g. InterruptIn), because mbed.h which we removed has a using namespace mbed.

Please keep fixing errors until

mbed test -t <TOOLCHAIN> -m CY8CKIT_062S2_43012 --compile

succeeds.

LDong-Arm · 2020-08-06T14:07:41Z

@paul-szczepanek-arm
Okay, there are a few things we need to resolve:

Missing includes and namespaced class names in COMPONENT_CYW43XXX - my see inline comments above.
Create an mbed_lib.json for each of COMPONENT_CYW43XXX, TARGET_CY8C63XX and TARGET_STM32WB. If we don't do this, bare metal applications will fail to compile, because "ble" isn't listed in "requires" and isn't built, but our build system will still try to build connectivity/drivers/ble/TARGET_xxx whose mbed_lib.json are missing... See this.
The BLE_GattServer example includes the old path #include "ble/FunctionPointerWithContext.h" - we should either update that example (just remove that line - "BLE.h" provides that already), or just move the header out of internal. Why is that in internal but CallChainOfFunctionPointersWithContext.h isn't?
Some examples expect BLE.h to bring in classes (e.g. Gap) without having to include one header per API (e.g. Gap.h), but it's not the case anymore in this PR. In my opinion, the expectation is reasonable - those classes are core features of BLE. (See Note below.)
The error log @adbridge posted means headers have dependency issues. We should reduce/simplify interdependencies a bit. (See Note below.)

Note:
To solve the last two points, I personally propose this change which includes API headers (e.g. Gap.h) into BLE.h rather than forward-declaring them, and also reduces interdependencies a bit. Please have a look and discuss.

LDong-Arm

Trying to dismiss the previous review

mbed-ci · 2020-08-07T12:05:21Z

Jenkins CI Test : ❌ FAILED

Build Number: 2 | 🔒 Jenkins CI Job | 🌐 Logs & Artifacts

CLICK for Detailed Summary

jobs	Status
jenkins-ci/mbed-os-ci_unittests	✔️
jenkins-ci/mbed-os-ci_build-ARM	❌
jenkins-ci/mbed-os-ci_build-GCC_ARM	❌

mbed-ci · 2020-08-07T12:37:09Z

Jenkins CI Test : ❌ FAILED

Build Number: 3 | 🔒 Jenkins CI Job | 🌐 Logs & Artifacts

CLICK for Detailed Summary

jobs	Status
jenkins-ci/mbed-os-ci_unittests	✔️
jenkins-ci/mbed-os-ci_build-ARM	❌
jenkins-ci/mbed-os-ci_build-GCC_ARM	❌

LDong-Arm

LGTM. @adbridge Could you please trigger CI again? Hopefully it'll pass this time, thanks!

adbridge · 2020-08-07T14:49:53Z

CI started

adbridge · 2020-08-07T14:52:04Z

connectivity/FEATURE_BLE/include/ble/GattClient.h

@@ -111,7 +109,16 @@ class GattClient : public StaticInterface<Impl, GattClient> {
        virtual void onAttMtuChange(
            ble::connection_handle_t connectionHandle,
            uint16_t attMtuSize
-        )
+        ) {


Formatting is incorrect here, for functions opening { go on their own line

adbridge · 2020-08-07T14:53:22Z

connectivity/FEATURE_BLE/include/ble/GattServer.h

@@ -117,22 +114,20 @@ class GattServer {
        virtual void onAttMtuChange(
            ble::connection_handle_t connectionHandle,
            uint16_t attMtuSize
-        )
+        ) {


incorrect formatting

adbridge · 2020-08-07T14:54:52Z

connectivity/FEATURE_BLE/include/ble/SecurityManager.h

+namespace interface {
+#endif // !defined(DOXYGEN_ONLY)
+class SecurityManager
+{


adbridge · 2020-08-07T14:55:11Z

connectivity/FEATURE_BLE/include/ble/SecurityManager.h

@@ -339,7 +325,7 @@ class SecurityManager {
         * @param[in] connectionHandle connection connectionHandle
         * @param[in] passkey 6 digit passkey to be displayed
         */
-        virtual void passkeyDisplay(ble::connection_handle_t connectionHandle, const SecurityManager::Passkey_t passkey) {
+        virtual void passkeyDisplay(ble::connection_handle_t connectionHandle, const Passkey_t passkey) {


adbridge · 2020-08-07T14:55:24Z

connectivity/FEATURE_BLE/include/ble/SecurityManager.h

@@ -373,7 +359,7 @@ class SecurityManager {
         * @param[in] connectionHandle connection connectionHandle
         * @param[in] keypress type of keypress event
         */
-        virtual void keypressNotification(ble::connection_handle_t connectionHandle, SecurityManager::Keypress_t keypress) {
+        virtual void keypressNotification(ble::connection_handle_t connectionHandle, ble::Keypress_t keypress) {


adbridge · 2020-08-07T14:55:55Z

connectivity/FEATURE_BLE/include/ble/SecurityManager.h

+     *
+     * @return BLE_ERROR_NONE or appropriate error code indicating the failure reason.
+     */
+    ble_error_t setPrivateAddressTimeout(


why is this on multiple lines ?

adbridge · 2020-08-07T15:03:15Z

connectivity/FEATURE_BLE/libraries/TARGET_CORDIO/include/ble/internal/PalGattClientImpl.h

+    /**
+     * @see ble::PalGattClient::when_server_message_received
+     */
+    void when_server_message_received(


adbridge · 2020-08-07T15:03:51Z

connectivity/FEATURE_BLE/libraries/TARGET_CORDIO/include/ble/internal/PalGattClientImpl.h

+    /**
+     * @see ble::PalGattClient::when_transaction_timeout
+     */
+    void when_transaction_timeout(


formatting, others below here too

adbridge · 2020-08-07T15:09:39Z

connectivity/FEATURE_BLE/libraries/TARGET_CORDIO/source/PalGattClientImpl.cpp

+ble_error_t PalGattClient::get_mtu_size(
+    connection_handle_t connection_handle,
+    uint16_t& mtu_size
+) {


adbridge · 2020-08-07T15:10:44Z

connectivity/FEATURE_BLE/libraries/TARGET_CORDIO/source/PalGenericAccessServiceImpl.cpp

+
+ble_error_t PalGenericAccessService::get_peripheral_preferred_connection_parameters(
+    ble::Gap::PreferredConnectionParams_t& parameters
+) {


LDong-Arm · 2020-08-07T15:19:11Z

We've always had the entire FEATURE_BLE in .astyleignore, even though much of it is our own code. If we do want to get formatting right in this PR, we should edit .astyleignore and run astyle on code owned by us.
What do you think? @adbridge @paul-szczepanek-arm

Pull request has been modified.

LDong-Arm · 2020-08-07T15:23:12Z

.astyleignore

@@ -1,9 +1,10 @@
 ^BUILD
 ^cmsis
+^connectivity/drivers/ble
+^connectivity/FEATURE_BLE


Suggested change

^connectivity/FEATURE_BLE

^connectivity/FEATURE_BLE/libraries/TARGET_CORDIO/stack

^connectivity/FEATURE_BLE/libraries/TARGET_CORDIO_LL/stack

This is what I mean - external code only. See my discussion with Paul earlier. @adbridge

Or can we do another PR to fix this, since this one is already pretty big?

adbridge · 2020-08-07T15:23:26Z

If it's our code then we shouldn't be ignoring astyle....

adbridge · 2020-08-07T15:37:43Z

Ci started

mbed-ci · 2020-08-07T16:11:08Z

Jenkins CI Test : ✔️ SUCCESS

Build Number: 4 | 🔒 Jenkins CI Job | 🌐 Logs & Artifacts

CLICK for Detailed Summary

jobs	Status
jenkins-ci/mbed-os-ci_unittests	✔️
jenkins-ci/mbed-os-ci_build-ARM	✔️
jenkins-ci/mbed-os-ci_build-GCC_ARM	✔️
jenkins-ci/mbed-os-ci_dynamic-memory-usage	✔️
jenkins-ci/mbed-os-ci_cloud-client-pytest	✔️
jenkins-ci/mbed-os-ci_greentea-test	✔️

mbed-ci · 2020-08-07T16:57:15Z

Jenkins CI Test : ✔️ SUCCESS

Build Number: 5 | 🔒 Jenkins CI Job | 🌐 Logs & Artifacts

CLICK for Detailed Summary

jobs	Status
jenkins-ci/mbed-os-ci_unittests	✔️
jenkins-ci/mbed-os-ci_build-ARM	✔️
jenkins-ci/mbed-os-ci_build-GCC_ARM	✔️
jenkins-ci/mbed-os-ci_dynamic-memory-usage	✔️
jenkins-ci/mbed-os-ci_greentea-test	✔️
jenkins-ci/mbed-os-ci_cloud-client-pytest	✔️

mergify bot added the needs: work label Jul 28, 2020

ciarmcom added the release-type: patch Indentifies a PR as containing just a patch label Jul 28, 2020

ciarmcom requested review from donatieng, LDong-Arm and a team July 28, 2020 19:00

ciarmcom added the needs: review label Jul 28, 2020

mergify bot removed the needs: review label Jul 29, 2020

LDong-Arm reviewed Jul 31, 2020

View reviewed changes

paul-szczepanek-arm mentioned this pull request Aug 3, 2020

Fix cancel connect #13390

Merged

paul-szczepanek-arm force-pushed the ble-streamlining branch from 953b33b to 4fe6928 Compare August 4, 2020 12:47

LDong-Arm reviewed Aug 4, 2020

View reviewed changes

paul-szczepanek-arm requested a review from LDong-Arm August 5, 2020 15:47

LDong-Arm reviewed Aug 5, 2020

View reviewed changes

LDong-Arm approved these changes Aug 5, 2020

View reviewed changes

adbridge added needs: CI and removed needs: work labels Aug 6, 2020

mergify bot added needs: work and removed needs: CI labels Aug 6, 2020

LDong-Arm reviewed Aug 6, 2020

View reviewed changes

LDong-Arm suggested changes Aug 6, 2020

View reviewed changes

adbridge added the needs: review label Aug 7, 2020

mergify bot added needs: work and removed needs: review needs: CI labels Aug 7, 2020

paul-szczepanek-arm added 3 commits August 7, 2020 14:20

make include path backwards compatible

ec4cbb7

backwards compatiblity with mbed build tools

b6db853

move non-api heders into common

f6a8610

LDong-Arm approved these changes Aug 7, 2020

View reviewed changes

adbridge previously requested changes Aug 7, 2020

View reviewed changes

braces always on newline

8bdd6bc

LDong-Arm reviewed Aug 7, 2020

View reviewed changes

braces on newlines

6aac3ac

adbridge approved these changes Aug 10, 2020

View reviewed changes

adbridge merged commit a6730ad into ARMmbed:master Aug 10, 2020

adbridge removed the needs: work label Aug 10, 2020

paul-szczepanek-arm mentioned this pull request Aug 10, 2020

Support to BlueNRG_2 BLE component #13246

Merged

LDong-Arm mentioned this pull request Aug 10, 2020

Move BlueNRG-MS driver into connectivity/drivers/ble/FEATURE_BLE #13409

Merged

mbedmain added release-version: 6.2.1 Release-pending and removed release-type: patch Indentifies a PR as containing just a patch Release-pending labels Aug 16, 2020

	^connectivity/FEATURE_BLE
	^connectivity/FEATURE_BLE/libraries/TARGET_CORDIO/stack
	^connectivity/FEATURE_BLE/libraries/TARGET_CORDIO_LL/stack

BLE streamlining #13365

BLE streamlining #13365

Uh oh!

Conversation

paul-szczepanek-arm commented Jul 28, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary of changes

Impact of changes

Migration actions required

Documentation

Pull request type

Test results

Reviewers

Uh oh!

paul-szczepanek-arm commented Jul 28, 2020

Uh oh!

ciarmcom commented Jul 28, 2020

Uh oh!

LDong-Arm left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

LDong-Arm Jul 31, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

paul-szczepanek-arm commented Aug 4, 2020

Uh oh!

LDong-Arm commented Aug 4, 2020

Uh oh!

mergify bot commented Aug 4, 2020

Uh oh!

paul-szczepanek-arm commented Aug 4, 2020

Uh oh!

LDong-Arm left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

LDong-Arm left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

LDong-Arm left a comment

Choose a reason for hiding this comment

paul-szczepanek-arm commented Jul 28, 2020 •

edited

Loading

LDong-Arm Jul 31, 2020 •

edited

Loading

LDong-Arm Aug 6, 2020 •

edited

Loading

LDong-Arm commented Aug 6, 2020 •

edited

Loading