Skip to content

Reactive rewrite continuation #23

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

Open
wants to merge 13 commits into
base: reactive-rewrite
Choose a base branch
from

Conversation

MonsieurRz
Copy link
Contributor

@MonsieurRz MonsieurRz commented Apr 6, 2025

continuation on #17

  • flutter upgrade:
  • flutter 3 compatible
  • android build migration ( gradle 8.5 & kotlin 1.9.10)
  • ios build migration (cannot assess whether that is necessary)
  • feature migration:
  • permission
  • ble scanning
  • ble connection
  • reimplement legacy methods for the Ergometer class
  • unit tests:
  • Ergomenter class testing

@MonsieurRz
Copy link
Contributor Author

MonsieurRz commented Apr 6, 2025

( #moved above)

@MonsieurRz
Copy link
Contributor Author

Looks finished on the feature side.
Next I am planning on mocking FlutterReactiveBle with Mockito for testing.
Seems tricky.

I do not own a Mac so I cannot test iOS, help is appreciated 🙂.

@MonsieurRz MonsieurRz marked this pull request as ready for review April 16, 2025 19:56
@MonsieurRz
Copy link
Contributor Author

Ready for reviews
@MoralCode if you can build ios I would be interested in your help.

@MonsieurRz MonsieurRz mentioned this pull request Apr 16, 2025
7 tasks
@MoralCode
Copy link
Collaborator

Sounds good! I'll try and review this when i can, but its been a while since my headspace was focused on this codebase (and most of the projects in this GH org)

If you have a chance to write up a summary of what was changed/why (and/or maybe update the example code in the README), especially as it relates to any breaking changes, that would probably help me get up to speed with testing this out.

I can tell you in advance that the biggest things ill be looking for when i get a chance to review this in detail are going to be figuring out how to make the transition for any existing users as easy as possible (i.e. if its possible to implement the old functions using the new ones (or at least have them not cause an intentional crash) that may be useful as well

This was referenced Apr 17, 2025
@MonsieurRz
Copy link
Contributor Author

MonsieurRz commented Apr 17, 2025

Quick summary:

  • Repository was migrated to build on a more recent version (android only):
    • permission handling and (android) requirements are updated (recent android are providing specific bluetooth permissions instead of just location)
    • the repository was upgraded to handle more recent build for Flutter and Android build:
Migration to a flutter 3 android compatible config.
graddle bump from version 6.7 to 8.5
kotlin bump from version 1.6.21 to 1.9.10
permission_handler upgrade from 10.2.0 to 11.1.0
  • The provided features are based on the last release (touching mostly lib/models.ergometer.dart):
    • Ergomenter.monitorForData() is added to later match the current OpenErgView's state.
    • Ergometer.connectAndDiscover() is simplified and combined with ConnectionState
    • Ergomenter.monitorConnectionState() is now deprecated in favor of getter Ergomenter.getMonitorConnectionState
    • Ergomenter.disconnectOrCancel() FlutterReactiveBle does not provide a disconnection method, destroying the ble object is the practice
  • Tests regarding the Ergometer class using FlutterReactiveBle mock:
    • getMonitorConnectionState test correct translation of statuses.
    • connectAndDiscover test connection
  • Other changes:
    • Ergometer "ble" optional argument can be passed to instantiate FlutterReactiveBle class (or passing the Mock class)
    • example app title is renamed "c2bluetooth example" (previously: "hello")
    • fixed most workout tests (unrelated to the reactive rewrite though)

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.

2 participants