Feature/esp provision provider #39
Conversation
…rking, lots of cleanup to do
…tes, taking changing RSSI into account and continuing scan until explicitly stopped.
…= when scan has been asked explicitly)
# Conflicts: # ORLib/build.gradle
Miggets7
left a comment
Miggets7
left a comment
There was a problem hiding this comment.
Some minor things to be discussed.
Overall it looks very well structured!
ORLib/src/main/java/io/openremote/orlib/service/espprovision/DeviceConnection.kt
Show resolved
Hide resolved
ORLib/src/main/java/io/openremote/orlib/service/espprovision/DeviceConnection.kt
Outdated
Show resolved
Hide resolved
ORLib/src/main/java/io/openremote/orlib/service/espprovision/DeviceRegistry.kt
Show resolved
Hide resolved
ORLib/src/main/java/io/openremote/orlib/service/espprovision/DeviceRegistry.kt
Outdated
Show resolved
Hide resolved
ORLib/src/main/java/io/openremote/orlib/service/espprovision/ORConfigChannel.kt
Show resolved
Hide resolved
There was a problem hiding this comment.
Should this be committed? I would say it wil be generated locally when working on the project?
There was a problem hiding this comment.
I tried a bit but couldn't get the gradle build to work.
I will give it another go.
|
Thanks, I'll take a look at those, maybe we can have a quick call next week |
… permissions have been requested to user (if required) (GitHub issue #40)
Miggets7
left a comment
Miggets7
left a comment
There was a problem hiding this comment.
Just a few questions, overall it looks very good!
There was a problem hiding this comment.
Not my expertise, but is it something we cannot test?
There was a problem hiding this comment.
I wanted to implement test, as I have on iOS side, but with my limited understanding of Kotlin and co-routines and I did not manage to come up with code that could handle the async nature of the provider.
Created #44 to address this later but I'd like to merge this PR even without tests for now.
There was a problem hiding this comment.
Shouldn't the proto file be present somewhere?
There was a problem hiding this comment.
In the end, I managed to generate this class from the protobuf spec, latest commit performs that and basically fixes #43
Miggets7
left a comment
Miggets7
left a comment
There was a problem hiding this comment.
Nice work!
If I can be of any help with the testing part of the protobuf stuff, let me know,
No description provided.