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

Release version 3.0 #245

Open
6 tasks done
ttu opened this issue Feb 19, 2024 · 27 comments
Open
6 tasks done

Release version 3.0 #245

ttu opened this issue Feb 19, 2024 · 27 comments
Assignees

Comments

@ttu
Copy link
Owner

ttu commented Feb 19, 2024

Tasks before creating a release:

@ttu ttu self-assigned this Feb 19, 2024
@terop
Copy link
Contributor

terop commented Feb 19, 2024

I created a commit for item two in the above list, it can be found at terop@757b441.
@ttu please take a look in case I missed something and if something should be change or improved.

@ttu
Copy link
Owner Author

ttu commented Feb 24, 2024

Thanks, commit looks good. I will leave comments if something comes to mind that is missing.

I was thinking that we should also drop support for 3.8 with this release. Its end of life is in October 2024, so by the time of the release, it won't be that far away. Based on the PyPI stats, it is not very widely used.

@terop
Copy link
Contributor

terop commented Feb 24, 2024

So is the plan to wait for October to make the release or also remove 3.8 support before official support ends (and not wait for October)?

@ttu
Copy link
Owner Author

ttu commented Feb 26, 2024

There is no need to wait for the official support to end. Based on current pypistats, most installations are from Python 3.10 and above.

There will be a final release for v2 with the fixes that have not yet been released, and after that, we can begin preparations for the v3 release.

@terop
Copy link
Contributor

terop commented Feb 26, 2024

Okay. I could then add another commit for the removal of Python 3.8.

@terop
Copy link
Contributor

terop commented Feb 27, 2024

I rebased my branch from master and now it contains the following commits.
Python 3.7 support removal: terop@b3658eb
Python 3.8 support removal: terop@0c94840

@terop
Copy link
Contributor

terop commented Mar 11, 2024

@ttu Could you kindly check the above commits?

@ttu
Copy link
Owner Author

ttu commented Apr 4, 2024

@terop sorry again for the delay. I think those commits already had pretty much all required changes. I found few mentions about default adapters that should be changed to Bleak, but after that I think those are good 👍

@terop
Copy link
Contributor

terop commented Apr 5, 2024

Thanks for the comments @ttu.
I fixed them as best I could and rebased the branch so now the commits are as follows:
Python 3.7 support removal: terop@133ee50
Python 3.8 support removal: terop@ec28465

@ttu
Copy link
Owner Author

ttu commented Jul 19, 2024

@terop sorry for the delay again. I finally had some time to get back to this.

I created two PRs; selecting default adapter and updated verification script. Could you create a PR for dropping Python 3.7 & 3.8 support?

I will still go through the functionality and test this with Linux to make sure we are not missing anything.

@terop
Copy link
Contributor

terop commented Jul 23, 2024

@ttu Sure and sorry for the delay.

Please find a PR to drop Python 3.7 and 3.8 at #251.

@terop
Copy link
Contributor

terop commented Oct 14, 2024

I created PR #256 for updating the examples.

@terop
Copy link
Contributor

terop commented Nov 9, 2024

Now all currently known tasks are done.

@ttu
Copy link
Owner Author

ttu commented Nov 10, 2024

Thanks 👍 I will make necessary updated to documentation and release a new version.

@ttu
Copy link
Owner Author

ttu commented Nov 23, 2024

@terop I was performing verifications before the release. I successfully ran verification.sh on macOS and Windows, but my Linux machine is experiencing some issues with Bluetooth. Could you run the verification from the master branch using Bleak and BlueZ on Linux?

@terop
Copy link
Contributor

terop commented Nov 24, 2024

@ttu Running verification.sh using BlueZ works fine but I get some error with Bleak. I will quickly check if I can figure what causes it.

@terop
Copy link
Contributor

terop commented Nov 24, 2024

The problem with Bleak seems to be the same I have been fighting with in my own application which uses ruuvitag-sensor async functions. My understanding is that you cannot run two Bleak asynchronous calls in the same process.

In verification.sh I think what happens is that once test_get_data_for_sensors_async() is started and the function returns the scanning process in Bleak does not stop. Therefore when test_get_data_for_sensors_async_with_macs() is called the bleak.exc.BleakDBusError: [org.bluez.Error.InProgress] Operation already in progress exception is raised.

I have spent some time looking at the code and trying to fix / workaround this but I have been unable to find a solution. In my own application I ended up separating the code in to two parts ("main.py" and "scan.py") where "main.py" calls "scan.py" which contains ruuvitag-sensor calls. Thus in case there is a need to scan RuuviTags multiple times the calls happens in separate processes and this error does not occur. I would very much like to get rid of this hack.

@terop
Copy link
Contributor

terop commented Nov 24, 2024

I did some more digging on this problem and it seems that https://github.com/ttu/ruuvitag-sensor/blob/master/ruuvitag_sensor/adapters/bleak_ble.py#L104 is possibly not called at the return from a Bleak async function. From the output of verification_async.py (see below) one can see that scanner.start() is called but the log message for the stop is not printed. Of course it is possible that the stop() is called but the log message is not printed. However my knowledge of Python async functions and Bleak API is insufficient to say what actually happens.

...
############################################
RuuviTagSensor.get_data_for_sensors_async
############################################
Get latest data for sensors. Stop with Ctrl+C.
Stops automatically in 5s
MACs: []
Bleak scanner started
<prints removed>
{'FB:15:98:DF:A0:A3': {'data_format': 5, ...}
OK
...

@ttu
Copy link
Owner Author

ttu commented Nov 26, 2024

@terop thanks for the info. It was pretty easy to debug the problem with that.

I noticed that GeneratorExit was not raised immediately when the async for loop was exited, which resulted in scanner.stop not being called immediately after the exit.

I made a change to explicitly close the generator after breaking the async for loop.

Can you try to execute verifications from this branch?
https://github.com/ttu/ruuvitag-sensor/tree/fix-close-generator-after-async-loop

@terop
Copy link
Contributor

terop commented Nov 26, 2024

@ttu Still broken with the same error. However I did notice the the branch you mentioned contains the same commits as master branch so I assume something is missing.

@ttu
Copy link
Owner Author

ttu commented Nov 26, 2024

@terop sorry, the actual commit was missing from the branch 😅 Can you try again?

@terop
Copy link
Contributor

terop commented Nov 26, 2024

@ttu Now it works, thank you!

Could you commit the fix soon? I would like to use it to get rid of the hack described above.

@ttu
Copy link
Owner Author

ttu commented Nov 26, 2024

@terop thanks! Fix is now merged to master.

@terop
Copy link
Contributor

terop commented Nov 27, 2024

Thanks @ttu!

@terop
Copy link
Contributor

terop commented Nov 27, 2024

@ttu Unfortunately your commit from yesterday didn't fix the problem as I had hoped so I am asking in case you have any ideas how to fix this.

I have a async function which runs RuuviTagSensor.get_data_async() with a given set of RuuviTag MACs with asyncio.wait_for() having a timeout. In case one or more of the tags is not found the wait_for() call will fail with TimeoutError and the Bleak scanner is not stopped. Thus is there a way to stop the Bleak scanner in this situation so that get_data_async() could be called again?
Edit: I did manage to come up with a slightly less ugly workaround than before but I would like to not have it either.

@ttu
Copy link
Owner Author

ttu commented Nov 28, 2024

@terop Can you provide me an example code how you are calling get_data_ascync multiple times.

I suppose Linux has some limitations on using multiple instances of BleakScanner. I suppose this code doesn't work on Linux?

import asyncio
from typing import AsyncGenerator

import ruuvitag_sensor.log
from ruuvitag_sensor.ruuvi import RuuviTagSensor
from ruuvitag_sensor.ruuvi_types import MacAndSensorData

ruuvitag_sensor.log.enable_console()


async def process_data_from_iter(data_iter: AsyncGenerator[MacAndSensorData, None], source_name: str):
    async for data in data_iter:
        print(f"Data from {source_name}: {data[0]}")


async def main():
    data_iter1 = RuuviTagSensor.get_data_async()
    data_iter2 = RuuviTagSensor.get_data_async()

    # Process both iterators concurrently
    task1 = asyncio.create_task(process_data_from_iter(data_iter1, "Iterator 1"))
    task2 = asyncio.create_task(process_data_from_iter(data_iter2, "Iterator 2"))

    await asyncio.gather(task1, task2)


if __name__ == "__main__":
    asyncio.run(main())

Maybe for Linux I have to try to find a way to force close previous scanner if Bleak.get_data is called multiple times.

@terop
Copy link
Contributor

terop commented Nov 29, 2024

I don't think it is worth checking this problem any further as most of the time the tag is not found during a second scan either.

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

No branches or pull requests

2 participants