Skip to content
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

pybricks.common.BLE: Implement observe_enable() #269

Merged
merged 2 commits into from
Oct 15, 2024
Merged

Conversation

laurensvalk
Copy link
Member

@laurensvalk laurensvalk commented Oct 4, 2024

This allows users to toggle observing off and on so that they can alternate between observing and broadcasting.

This results in better stability on Technic Hub which can lock up when simultaneous advertising and broadcasting.

Fixes pybricks/support#1806

Also fix the remaining FIXME about the default broadcast channel while we are working on this code (first commit).

Usage:

hub.ble.observe_enable(True) # Enable observing (on by default)
hub.ble.observe_enable(False) # Disable observing

Can be combined with hub.ble.broadcast(data) and hub.ble.broadcast(None) to alternate between broadcasting and observing for better reliability on Technic Hub.

This isn't extensively tested yet, but making this available for others to try.


@NStrijbosch, @JJHackimoto

Broadcasting should only be possible when a channel is provided, just like observing defaults to no channels.
This allows users to toggle observing off and on so that they can alternate between observing and broadcasting.

This results in better stability on Technic Hub which can lock up when simultaneous advertising and broadcasting.

Fixes pybricks/support#1806
@coveralls
Copy link

Coverage Status

coverage: 56.826%. remained the same
when pulling 3a5d824 on observe_enable
into 4ff6e7b on master.

@laurensvalk
Copy link
Member Author

laurensvalk commented Oct 5, 2024

@@JJHackimoto, if you want to try this with blocks:

  1. Create a new python file called issue_1806_lib.py with the following contents:
from pybricks.tools import run_task

async def observe_enable_async(hub, enable):
    await hub.ble.observe_enable(enable)

def observe_enable(hub, enable):
    if run_task():
        return observe_enable_async(hub, enable)
    hub.ble.observe_enable(enable)
  1. Then you can use this into your block program as follows:

image

This will blink the light for 5 seconds based on the number it receives. Then observing is disabled for 5 seconds so it will eventually receive nothing (None) for 5 seconds, so the light is off.

Naturally, the Technic Hub still has the usual issue of not observing very well while still connected to the PC. So you may want to build a different example to test things.

  1. Here is the sending program I used:

image

@JJHackimoto
Copy link

JJHackimoto commented Oct 10, 2024

Sadly not working. I'm getting the following error in the issue_1806_lib.py file when trying to run my blocks code:

File "issue_1806_lib.py", line 4, in observe_enable_async
AttributeError: 'BLE' object has no attribute 'observe_enable'

I'm using PyBricks Beta (Checking for updates returns up to date)

@BertLindeman
Copy link
Contributor

Sadly not working. I'm getting the following error in the issue_1806_lib.py file when trying to run my blocks code:

File "issue_1806_lib.py", line 4, in observe_enable_async AttributeError: 'BLE' object has no attribute 'observe_enable'

I'm using PyBricks Beta (Checking for updates returns up to date)

Did you install the firmware for this update?
See artifacts for this update

@JJHackimoto
Copy link

Must have completely missed that! Thanks!

Happy to see the firmware installation bug on M1 Macs have been adressed, installing went smoothly on all three hubs!
I'll test the observing enable feature and report back.

@JJHackimoto
Copy link

JJHackimoto commented Oct 10, 2024

I can't seem to get any observing going after having called the new functions. It runs fine until I've disabled observing, then after enabling it again, it never observes again. Here's part of my code. Both of the print-functions in the "Broadcast" function runs, but after this, the "Hello" print no longer appears. It seems to get stuck at observing as if it was still disabled.
Any clues? I'll go ahead and create a simpler program if you cannot find anything obvious here.

Skärmavbild 2024-10-10 kl  18 20 22

@laurensvalk
Copy link
Member Author

It’s hard to say for sure since we don’t see the whole code, but your ‘hello’ loop ends when you set that loop condition to false, so this seems like the expected result.

@JJHackimoto
Copy link

Yes, you are absolutely correct. My bad.

It seems to be working now, I'll do some more testing in the coming days, but it surely looks promising :)
Quick question, how long do you think I should be broadcasting for to make sure the signal is picked up by the other hubs? Is there any reason why it wouldn't pick it up right away (unless I've turned off observing at that specific time ofc)? Also, for how long after broadcasting will the signal still be able to be picked up, or is there no delay like that?

@laurensvalk
Copy link
Member Author

For Technic Hubs, observing works best when disconnected from the PC. I don't know that there are any guarantees, but it should pick it up within half a second?

Especially if you also occasionally intentionally disable observing in this way. This is ultimately just a workaround to trade speed for better long term reliability in advanced creations like yours 🙂

Depending on the application specifics, I suppose you can find something that works well enough for you, but there might not be a generic solution that covers all cases in an optimal way.

It seems to be working now, I'll do some more testing in the coming days, but it surely looks promising :)

Nice!

@JJHackimoto
Copy link

JJHackimoto commented Oct 13, 2024

Thanks for the info! I didn't have any broadcasts that wasn't also observed so that works well!

However, I'm sorry to let you know that after about 10 minutes today, one of my hubs crashed. Same as before, no response and only resets when holding the power button.

All three hubs are flashed with the latest firmware for this fix and I'm using the loop in the screenshot so I don't think it should be able to broadcast and observe at the same time. I'm only sending booleans when broadcasting (all hubs do).
Skärmavbild 2024-10-13 kl  15 21 53

@laurensvalk
Copy link
Member Author

laurensvalk commented Oct 15, 2024

Can you please share your whole program that reproduces the issue? It's hard to say what could be wrong otherwise. Thanks!

In your example, I don't see broadcasting being disabled. Remember that broadcasting happens continuously until you choose broadcast(None). Above, it looks like observing and broadcasting are still happening at the same time, so the lockup is still expected to eventually occur.

@laurensvalk laurensvalk merged commit 3a5d824 into master Oct 15, 2024
38 checks passed
@laurensvalk
Copy link
Member Author

I've merged this so it can be part of today's beta release. We can still refine it further before the release as needed.

@dlech dlech deleted the observe_enable branch October 15, 2024 14:57
@JJHackimoto
Copy link

In your example, I don't see broadcasting being disabled. Remember that broadcasting happens continuously until you choose broadcast(None).

Oh, that's news to me. I had no idea broadcasting could also be turned off like this. I'll try this setup when I got some time (I guess this is how it would be done when using blocks). Thanks for letting me know. Will share my full program if this doesn't work.
Skärmavbild 2024-10-16 kl  21 11 15

@JJHackimoto
Copy link

Has this feature been implemented or do I still need to call the observe_enable external task (issue_1806_lib.py) for this to work? I have been putting this off for quite some time but I'm finally getting back into it. Last time, I kept receiving the value 0 instead of None when no recent value was broadcasted. I want to try again with the new firmware if an implementation has been made.

@laurensvalk
Copy link
Member Author

There is no dedicated block for it, so importing as you have sounds like a good option.

@JJHackimoto
Copy link

Ok thanks!

Here's my findings from today.

When broadcasting and observing a list, the default value is not "none", but 0. Broadcasting and observing a single variable has a default value of "none". Is this intended?

I have tested my programs multiple times during the day, and observing is very inconsistent. More than half the time, the broadcasted value won't be registered at all. If I connect my computer and print every single observation, they stay at "none", even when I know a broadcast has been sent. I'm broadcasting for a full second, and observation is done each 150ms. Worth noting, only the hubs that runs both broadcasting and observing are having this issue (maybe enabling broadcasting after being disabled is not instant?) The hub that only broadcasts can be observed by the other hubs all the time, no issues.

I'll share all my code in case someone can help me out.

For my observation function, I'm using a temporary variable to detect if the value observed is not "none". If so, that value is then applied to the actual variable. This is because the rest of the code would break if the variable was "none" when expected result is bool. My wish is that the variable observed should stay as last observed if no new observation was made.

If any part of the code needs explaining, then please let me know and I'll help. I feel like we are getting closer with this. No crashes were detected at all today!
pybricks-backup-2024-12-01_19-19-57.zip

@laurensvalk
Copy link
Member Author

Could you include screenshots of your code? You can use the camera icon on the right hand side and just drag the files in here. Thanks!

When broadcasting and observing a list, the default value is not "none", but 0. Broadcasting and observing a single variable has a default value of "none". Is this intended?

I don't really understand what you mean about the default value of a list having a value. A list is either empty or contains the values you have put in it.

For my observation function, I'm using a temporary variable to detect if the value observed is not "none". If so, that value is then applied to the actual variable. This is because the rest of the code would break if the variable was "none" when expected result is bool. My wish is that the variable observed should stay as last observed if no new observation was made.

I suppose this would make it harder to detect the absence of incoming data, but maybe it could make sense to have this "data deletion timeout" extended or make it user configurable.

I have tested my programs multiple times during the day, and observing is very inconsistent. More than half the time, the broadcasted value won't be registered at all. If I connect my computer and print every single observation, they stay at "none", even when I know a broadcast has been sent.

Combining broadcasting, observing and being connected to your PC is a bit too much for the Technic Hub to handle. Observing really only works well if disconnected. Maybe you could use the hub light to indicate if something was received?

@JJHackimoto
Copy link

Could you include screenshots of your code? You can use the camera icon on the right hand side and just drag the files in here. Thanks!

Sure! I've attached the screenshots of my current code.

I don't really understand what you mean about the default value of a list having a value. A list is either empty or contains the values you have put in it.

Well, if observing channel 4 (and nothing is broadcasting there), the observed value is "none". However, if observing and expecting a list of two variables, those two variables will instead get the value 0 (after unpacking the list). I expected them to also get the value "none".

I suppose this would make it harder to detect the absence of incoming data, but maybe it could make sense to have this "data deletion timeout" extended or make it user configurable.

Yeah, I'm not sure what a good implementation would be for this. It could also easily get confusing, so maybe it's fine as is.

Combining broadcasting, observing and being connected to your PC is a bit too much for the Technic Hub to handle. Observing really only works well if disconnected. Maybe you could use the hub light to indicate if something was received?

Good idea! I'll try that and report back.

Sensor Hub:
pybricks_program_2024-12-02T16_26_53
Drive Hub:
pybricks_program_2024-12-02T16_26_58
Distributor Hub:
pybricks_program_2024-12-02T16_27_02

@JJHackimoto
Copy link

JJHackimoto commented Dec 2, 2024

I just tested again for about 15 minutes, and the hub light went cyan each time the hub sends a message, but watching the other hubs (looking for a green light) made it clear that the message got through only about 50% of the time or less. I couldn't see any pattern as to when it worked and when it didn't.

@laurensvalk
Copy link
Member Author

Thanks. I think I'd need to try running this.

If I understand correctly you have two hubs communicating bidirectionally and a third hub that is broadcasting to the first two also. I'll try setting up a small example to see how well this can work.

@JJHackimoto
Copy link

Yes that's correct. Thanks!

@JJHackimoto
Copy link

Hi! Have you had time to test? :)
I made a youtube video about a day ago showcasing my project (And someone at Pybricks commented!), but it was difficult to make since I had to do multiple takes until it all worked. I made it look like it works since I believe it soon will :)

@laurensvalk
Copy link
Member Author

I started giving it a go, but quickly realized it gets pretty cumbersome at the user level. It should still work though.

So I've been thinking it might make sense to toggle between observing and broadcasting at the firmware level instead.

Since we know now that this works around the freezing, this might be worth doing and then call it good on bluetooth for this hub :)

We are already doing something like this to constantly restart observing, so having a toggle like this shouldn't change the overal driver code by that much.

@JJHackimoto
Copy link

Alright that sounds awesome to me! So basically it would take care of itself under the hood and all I do is to broadcast and observe at the same time like I used to do? (When this is implemented ofc)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Feature] Make it possible to stop observing
4 participants