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

App intents #859

Closed
wants to merge 8 commits into from
Closed

App intents #859

wants to merge 8 commits into from

Conversation

RCGV1
Copy link
Member

@RCGV1 RCGV1 commented Aug 10, 2024

What changed?

Modified the BLEManager class to make it a singleton to access from anywhere. Added two App Intents: Channel Message and Send Waypoint

Why did it change?

Allowing users to utilize apple shortcuts to greatly customize their Meshtastic experience. Example: a custom weather-sending bot built with no coding experience and just iOS shortcuts. Also will enable Siri functionality and widgets for future development.

How is this tested?

I tested sending waypoints from the shortcut on my phone and tested sending messages. The normal functionality of ble works after my BLEManager change

Screenshots/Videos (when applicable)

IMG_1646
IMG_1645
IMG_1640

Checklist

  • My code adheres to the project's coding and style guidelines.
  • I have conducted a self-review of my code.
  • I have commented my code, particularly in complex areas.
  • I have made corresponding changes to the documentation.
  • I have tested the change to ensure that it works as intended.

@thebentern
Copy link
Collaborator

Phenomenal work, Ben!

@garthvh
Copy link
Member

garthvh commented Aug 10, 2024

Can you make a branch in the repo and not on your personal account?

@@ -11,6 +11,7 @@ import OSLog
// Meshtastic BLE Device Manager
// ---------------------------------------------------------------------------------------
class BLEManager: NSObject, CBPeripheralDelegate, MqttClientProxyManagerDelegate, ObservableObject {
static var shared: BLEManager! // Singleton instance
Copy link
Contributor

@72A12F4E 72A12F4E Aug 10, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have some long-term testability concerns about making BLEManager a Singleton.

Could we use @Environment and dependency injection to the places where it's used instead?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@72A12F4E IMO something needs to be done about BLEManager being transient though. For instance, currently the Share Location functionality is capable of spinning up multiple timers during consecutive connect / disconnect cycles because of the lifecycle of that BLEManager object.
To repro: Take a device with location sharing on the default interval, connect and disconnect multiple times in succession.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps the timer stuff needs to be up an abstraction layer? Not sure what the idiomatic way of doing this is

@garthvh
Copy link
Member

garthvh commented Aug 20, 2024

Closing in favor of #866

@garthvh garthvh closed this Aug 20, 2024
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 Request: Add App Intents for CarPlay, Shortcuts and Siri control
4 participants