-
Notifications
You must be signed in to change notification settings - Fork 7
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Feat emulated device #974
Feat emulated device #974
Conversation
The new adapter extends asyncio based GenericBLEDeviceAdapter.
477be0b
to
d3a80e7
Compare
|
||
#FIXME: Raise an exception here | ||
|
||
async def request_scan(self, tag: str, active: bool, delegate: BLEScanDelegate = None): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@DimaWittmann The BLEScanDelegate concept I believe is old and can be removed. Previously scan events came via callbacks made on the BLEScanDelegate object passed in but we moved to centralizing that through the events
in the async central so this shouldn't be needed anymore and can just be removed completely.
Haven't had a chance to experiment yet, but does it gracefully handle someone removing a bled112 dongle? See this: https://app.zenhub.com/workspaces/archfx-nodes-5d8141feed817a000174f16b/issues/iotile/arch_node_tasks/114 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since this is a large feature PR, I will defer final approval to Tim B.
I will approve when I see test output and the final throughput results from Tim F.
I've been unable to get the plugin to work, either on the AP or on my laptop. Here are the commands I'm running. Is there anything that I'm missing here, @DimaWittmann?
|
@timothyfroehlich It looks like there may be an importerror causing the device adapter to be ignored, can you turn up the logging and see if
|
@@ -9,6 +9,9 @@ | |||
install_requires=[ | |||
"iotile-core>=5.0.0,<6" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missed requirement for typing_extensions
After installing
|
@timothyfroehlich Looks like that error is caused by using a released The error should have been fixed by this part of the PR: |
Well, that fixed it. I would have assumed that running |
Seems to be working, but I'm getting a significant number of these errors: |
Performance-wise, with just 20 simulated Pods (175 broadcasts per second), I saw a significant amount of dropped packets. For my AP with the async bled, it dropped 77/519 from one SimPod. My AP running the latest released coretools and bled transport only dropped 19/521 from the same SimPod. With 35 SimPods (320 broadcasts per second) the Async bled AP didn't fall behind or start queueing up packets, but it dropped almost half of the packets it received., and at one point missed an update from a Pod that updates it's value every second. So I believe it was mentioned that this current build is less performant than the original code, and my tests show this as well. |
@timothyfroehlich @timburke Do you think that we should leave this PR open or should we merge it now? |
(Reposting since some of my comment got lost somehow)
My results output isn't very github friendly (I'm just printing graphs at the moment) but if it'd be useful then I could post something more detailed.
Also, this is still in draft and @DimaWittmann said there's more work to be done. @DimaWittmann Let me know if you'd like me to run more performance tests for you, I have a branch set up for my AP so it's easy for me to try the latest. |
This is a great comparison. What I would like to do, is put the brakes on this PR and set it aside. Since there is a significant decrease in performance we should suspend work on it until we have time to dig deeper into debugging the plugin. |
Overview
This PR integrates a new async ble plugin and blelib.
Tim Burke has created asyncio based ble plugin that should increase the throughtput and get rid of multiple wrapperers inside of current thread based ble plugin. I integrated code from another branch https://github.com/iotile/coretools/tree/feat-async-wrapper as well.
At the moment only scanning is fully implemented. Major functionality is in place but some parts of code are missed.
#How to
In order to run a new plugin user should install iotile-transport-bled112 and iotile-transport-blelib modules from this PR.
Instantiate the hardware manager with the new plugin:
iotile hw --port=async_bled112
To run iotile-gateway with new pluging a user needs to update
name
entry in config.