buttplugio / buttplug-android

Java Implementation of Buttplug server for Android
Other
11 stars 3 forks source link

not compatible with androidx-only builds #2

Open dreadhonk opened 4 years ago

dreadhonk commented 4 years ago

the library is still using the legacy android support compat stuff. in my build based on the current SDK, this doesn’t work.

I have patches which port the library to androidx, would it make sense to get that merged?

if there's interest in the patches just let me know and I'll set up a PR.

qdot commented 4 years ago

Wow. Uh. I had no idea anyone was even still looking at this library. I really gotta be better about archiving this stuff. >.>

I have no plans on updating this repo myself (I didn't write it in the first place), and it's about to become massively out of sync with the new Rust library we're centralizing on (https://github.com/buttplugio/buttplug-rs), which I'm planning on having a Android port for in the near-to-middle future. Basically it'll either have a Java or Kotlin layer on top of it, and will be able to access android bluetooth. I've already got iOS compiling (though no Swift/ObjC binding layer yet either).

Anyways, that means this version will stop working with any system involving the new spec (Buttplug Protocol version compatibility is how the system stays coherent), which is pretty involved to implement.

Sooo I guess updating this repo kinda depends on what you're using it for? Like, if you're just building this for a one-off thing for your own app, feel free to fork and update as needed, but I'm not sure I'd recommend trying to keep this up to date with our reference library.

BTW, if you have opinions/needs in a Buttplug Android library, I'd love to talk more. :)

dreadhonk commented 4 years ago

ha, thanks for the quick reply! and thanks for not archiving this (yet) -- I might have lost interest if I had not found this library or had found it to be discontinued right away. now I'm hooked ;).

my current goal is to implement a generic toy controller app based on buttplug. I have some fun features in mind (see below).

thoughts on the library

regarding the upcoming rust-based library, is there a timeframe for android compatibility? for now, I'm embedding the server either way so I don't care much about the wire protocol; the key thing is that I can use it. regarding the feature set of the library, I thought a bit about this.

event-based and stateless

I think an event-based, stateless library would be the most fun thing to use. I'd like to get events on device appearance/disappearance. those events should carry all device meta-information as well as a unique (and ideally, stable) identifier (so some kind of hardware address, for bluetooth that's easy, not sure if USB/serial/whatnot based things have some kind of serial number? otherwise the library has to make something up).

then any operation on the device (query info, send commands etc.) should only need the device ID plus whatever parameters the command needs. if I try to execute a command not supported by the device, I should get an error back. if the device has disconnected in the meantime, I should get an error back. and so on.

the device metadata should cover:

generic

whenever possible, generic APIs for accessing device functions should be made available (e.g. setting motor speed). I think this is already mostly the case.

android specifics

the library (or the android backend) must support non-bluetooth-LE scanning. on my lineage OS device, bluetooth LE scanning does not work at all. it does not find any devices. however, a normal scan finds my lush 2 without issues (after I disabled privacy guard, too... this is a known bug. it doesn't affect LE scanning though; I patched non-LE scanning into bluetooth-android).

the backend should probably not manage the android permissions by itself, but leave that up to the host app. the reason for this is that having to pass down an Activity as context so that the device manager can bring up the permission request is a blocker for running the device manager in a foreground service (which is kind of required if you want the thing to continue to work even if the app is not active right now).

random language / api notes

kotlin is much preferred over java. also, do kotlin-based coroutines make sense on android? if so, they would probably be an amazing interface.

error handling should be sane: any asynchronous operation should return an error if it fails and not silently swallow it. errors which are not directly tied to an operation should be reported via events (e.g. if scanning cannot start due to lack of permissions).

I'd also like to have thread-safety or at least documentation on the threading properties of the library. I think (I haven't dug into all the code yet), some event handlers in buttplug-android are being called from random (executor) threads, which is annoying if you don't know about it. at the same time, either the library needs to be fully thread-safe, or it needs to make it clear which parts are (not).

unfortunately, I'm neither well-versed in rust nor am I familiar with embedding native code in android apps, so I don't think I'll be able to help on that front.

mind also that I only worked with this stuff for like a week or so, and due to the patches needed to make the buttplug-android library even find the Lush 2.


either way: I fully understand not wanting to maintain this legacy codebase if it's possible to embed the rust thing in kotlin in some way. go for it! less code duplication is always appreciated.

my takeaway is: I should try to abstract the buttplug implementation in my app as far as possible (I wanted to do that either way because I also want to allow remote control over non-buttplug protocols). then once the rust-based library hits, I can port my code. and until then I won't waste any time supporting the non-embedded client, since things are going to change there specifically.

blackspherefollower commented 4 years ago

Not that I'm encouraging following the pure java route too far, but a while back another dev did some work on this repo in order to get it working with Unity (mostly compatibility fixes I think): https://github.com/JaviTechnologies/ButtplugTest/tree/master/ServerCode

qdot commented 4 years ago

Just a heads up, don't want you to think I'm being rude/annoyed, but I'm gonna be kinda terse here 'cause most of this is already answered elsewhere. :)

regarding the upcoming rust-based library, is there a timeframe for android compatibility? for now, I'm embedding the server either way so I don't care much about the wire protocol; the key thing is that I can use it. regarding the feature set of the library, I thought a bit about this.

There are 2 requirements for getting android up and running:

Unfortunately I don't have a great timeframe on either of those, but for now I'd call it months, because I'm still finishing the base rust library, then C# (if not others) will precede Java since our main library currently is C#. That said, the work for C# FFI from rust should be reusable for all of our other FFI.

event-based and stateless

I think an event-based, stateless library would be the most fun thing to use. I'd like to get events on device appearance/disappearance. those events should carry all device meta-information

We already do that. See our protocol flow in the spec:

https://buttplug-spec.docs.buttplug.io/architecture.html#stages

as well as a unique (and ideally, stable) identifier (so some kind of hardware address, for bluetooth that's easy, not sure if USB/serial/whatnot based things have some kind of serial number? otherwise the library has to make something up).

Right now, you get a uint32 ID that related to the device. We don't normally send up universally unique identifiers to the client.

then any operation on the device (query info, send commands etc.) should only need the device ID plus whatever parameters the command needs. if I try to execute a command not supported by the device, I should get an error back. if the device has disconnected in the meantime, I should get an error back. and so on.

We do this already. This is covered in the spec.

the device metadata should cover:

* which features does it have (number and use-case of motors, for example)

* human-friendly name

Both covered in the spec, see DeviceAdded/DeviceList messages

* limits: at which rate can I update motor speeds? which granularity does the motor speed have? (this is required to employ filtering of my output to prevent it from flapping quickly between two motor states)

Covered in Protocol v2, see https://github.com/buttplugio/buttplug/issues/91

generic

whenever possible, generic APIs for accessing device functions should be made available (e.g. setting motor speed). I think this is already mostly the case.

Covered in the spec, see Generic messages. https://buttplug-spec.docs.buttplug.io/generic.html#stopdevicecmd

the library (or the android backend) must support non-bluetooth-LE scanning. on my lineage OS device, bluetooth LE scanning does not work at all. it does not find any devices. however, a normal scan finds my lush 2 without issues (after I disabled privacy guard, too... this is a known bug. it doesn't affect LE scanning though; I patched non-LE scanning into bluetooth-android).

What does "non-bluetooth LE scanning" mean? These devices support BLE, there isn't another kind of scanning I'm aware of?

the backend should probably not manage the android permissions by itself, but leave that up to the host app. the reason for this is that having to pass down an Activity as context so that the device manager can bring up the permission request is a blocker for running the device manager in a foreground service (which is kind of required if you want the thing to continue to work even if the app is not active right now).

Ok this is something we'll have to deal with on the library side, because app code will probably look like:

app (Java/Kotlin) -> Buttplug (rust) -> Bluetooth (JNI)

So I'm not quite sure where things will need to be handled. I'm modeling my bluetooth work off the webbluetooth work in the Servo browser, so it's at least been done before, though whether that was done right is a good question.

kotlin is much preferred over java. also, do kotlin-based coroutines make sense on android? if so, they would probably be an amazing interface.

No idea, haven't done any kotlin research yet, I just know people I trust about languages like it. :)

error handling should be sane: any asynchronous operation should return an error if it fails and not silently swallow it. errors which are not directly tied to an operation should be reported via events (e.g. if scanning cannot start due to lack of permissions).

Spec, see error messages, state flow.

I'd also like to have thread-safety or at least documentation on the threading properties of the library. I think (I haven't dug into all the code yet), some event handlers in buttplug-android are being called from random (executor) threads, which is annoying if you don't know about it. at the same time, either the library needs to be fully thread-safe, or it needs to make it clear which parts are (not).

What part of FEARLESS GOD DAMN CONNCURRENCY do you not get? :) (It's in rust. It'll be thread safe in the rust parts).

unfortunately, I'm neither well-versed in rust nor am I familiar with embedding native code in android apps, so I don't think I'll be able to help on that front.

Yeah I've got resources who are so that should be ok.

either way: I fully understand not wanting to maintain this legacy codebase if it's possible to embed the rust thing in kotlin in some way. go for it! less code duplication is always appreciated.

Yup, that's why the rust rewrite. It FFI's nicely pretty much everywhere, versus, say, our C# implementation where your options were "Xamarin or fuck off". :)

my takeaway is: I should try to abstract the buttplug implementation in my app as far as possible (I wanted to do that either way because I also want to allow remote control over non-buttplug protocols). then once the rust-based library hits, I can port my code. and until then I won't waste any time supporting the non-embedded client, since things are going to change there specifically.

The hope is that most of the FFI built on top of rust will look the same as what we've got out now. Problem here is that I didn't write this particular implementation, nor did I ever use it, so I can't say the same for here. That said, it also means I currently have no set opinions on what this should look like, so there's nothing that's unchangable up front.

dreadhonk commented 4 years ago

thanks for taking the time to write this lengthy essentially RTFM post.

I do have a few quick follow ups, and I'm going to simply elide the parts of your reply which I don't need to follow up on. also note that my comments were mostly about the API.

as well as a unique (and ideally, stable) identifier (so some kind of hardware address, for bluetooth that's easy, not sure if USB/serial/whatnot based things have some kind of serial number? otherwise the library has to make something up).

Right now, you get a uint32 ID that related to the device. We don't normally send up universally unique identifiers to the client.

is this ID stable across server restarts and device reconnects? if not, this would be a problem for me. if a user has multiple devices of the same make & model, I need to be able to distinguish them, even when the disconnect and reconnect and across app restarts. I want to allow the user to configure different inputs for the different outputs, and having to re-do that all the time would be kind of meh.

it doesn't have to be the plaintext bluetooth address, but something which allows me to recognize the same device on the same server when I see it again.

* limits: at which rate can I update motor speeds? which granularity does the motor speed have? (this is required to employ filtering of my output to prevent it from flapping quickly between two motor states)

Covered in Protocol v2, see buttplugio/buttplug#91

:+1:

the library (or the android backend) must support non-bluetooth-LE scanning. on my lineage OS device, bluetooth LE scanning does not work at all. it does not find any devices. however, a normal scan finds my lush 2 without issues (after I disabled privacy guard, too... this is a known bug. it doesn't affect LE scanning though; I patched non-LE scanning into bluetooth-android).

What does "non-bluetooth LE scanning" mean? These devices support BLE, there isn't another kind of scanning I'm aware of?

the library currently uses the BluetoothAdapter.startLeScan() method. this method does not yield any devices on my phone. I have no idea why. if I use BluetoothAdapter.startDiscovery() (+ the corresponding listeners), I get my device listed.

the backend should probably not manage the android permissions by itself, but leave that up to the host app. the reason for this is that having to pass down an Activity as context so that the device manager can bring up the permission request is a blocker for running the device manager in a foreground service (which is kind of required if you want the thing to continue to work even if the app is not active right now).

Ok this is something we'll have to deal with on the library side, because app code will probably look like:

app (Java/Kotlin) -> Buttplug (rust) -> Bluetooth (JNI)

So I'm not quite sure where things will need to be handled. I'm modeling my bluetooth work off the webbluetooth work in the Servo browser, so it's at least been done before, though whether that was done right is a good question.

I mean if the library simply knows to check for permissions and fail cleanly if they're missing, that's good enough I think. though that makes it a bit tricky if an app wants to support local and remote servers, since it wouldn't need permissions for remote servers ...

I'd also like to have thread-safety or at least documentation on the threading properties of the library. I think (I haven't dug into all the code yet), some event handlers in buttplug-android are being called from random (executor) threads, which is annoying if you don't know about it. at the same time, either the library needs to be fully thread-safe, or it needs to make it clear which parts are (not).

What part of FEARLESS GOD DAMN CONNCURRENCY do you not get? :) (It's in rust. It'll be thread safe in the rust parts).

I'm not sure I fully understand this statement.

my takeaway is: I should try to abstract the buttplug implementation in my app as far as possible (I wanted to do that either way because I also want to allow remote control over non-buttplug protocols). then once the rust-based library hits, I can port my code. and until then I won't waste any time supporting the non-embedded client, since things are going to change there specifically.

The hope is that most of the FFI built on top of rust will look the same as what we've got out now. Problem here is that I didn't write this particular implementation, nor did I ever use it, so I can't say the same for here. That said, it also means I currently have no set opinions on what this should look like, so there's nothing that's unchangable up front.

I trust that you folks can do sane API design. I don't worry about that, and if I have to change things on my end, that's fine.

qdot commented 4 years ago

is this ID stable across server restarts and device reconnects? if not, this would be a problem for me. if a user has multiple devices of the same make & model, I need to be able to distinguish them, even when the disconnect and reconnect and across app restarts. I want to allow the user to configure different inputs for the different outputs, and having to re-do that all the time would be kind of meh.

Depends. In C#, it's stable within a process, i.e. if you disconnect/reconnect a device, it'll keep the ID, but if the process goes down and comes back up, new ID. In JS, it's new every time. So it's kinda implementation dependent.

That said, the point you make about configuring inputs is good, and something I've gotta think about for Intiface (our UI frontend to buttplug), so this could possibly grow out of that as a config file or something.

A quick overview on that since it's not covered in the spec, because it's not really part of the protocol. All of our device definitions are loaded from a JSON file. A copy of this file is compiled into the library, but can also be loaded from outside if need be. The newest unreleased version is here:

https://github.com/buttplugio/buttplug-device-config/tree/device-attributes

I recommend reading the YAML version, JSON is just easier to parse so we convert for the library.

That file contains all of the information we need to identify devices on scan, as well as the features they have available in relation to protocol messages. We could further customize this for specific device settings, though it'd only work for devices that have unique IDs, i.e. USB, HID, and Serial might be an issue.

the library currently uses the BluetoothAdapter.startLeScan() method. this method does not yield any devices on my phone. I have no idea why. if I use BluetoothAdapter.startDiscovery() (+ the corresponding listeners), I get my device listed.

Oof, yeah, this is why I'm not looking forward to android support. Some of the Xamarin libraries I was using also mentioned that Android support is a very specific nightmare.

All of this would be covered in our lower level bluetooth library I think, so I'll try to keep it in mind once we start implementation there.

I mean if the library simply knows to check for permissions and fail cleanly if they're missing, that's good enough I think. though that makes it a bit tricky if an app wants to support local and remote servers, since it wouldn't need permissions for remote servers ...

The way Buttplug is built, applications just use the Client API and it should be opaque whether the server is local/remote, outside of issues like latency. So it'd just come back as a connection/scan error from the client in certain cases, which it'd be up to the app UI to handle.

What part of FEARLESS GOD DAMN CONNCURRENCY do you not get? :) (It's in rust. It'll be thread safe in the rust parts).

I'm not sure I fully understand this statement.

Sorry, Rust joke. :)

Rust has specific structures in the language to maintain thread safety. Buttplug-rs uses async rust, which means we only sometimes know what thread we're on, but we're guaranteed by the language that accesses will be safe. I'll be writing up more on the Rust architecture soon, but it'll require some background in the language to understand it.

I trust that you folks can do sane API design. I don't worry about that, and if I have to change things on my end, that's fine.

Yeah, but we'll still need input on the Android side, as I'm sure the platform has idioms at the app level that we may or may not have to deal with, so I'm definitely open to input as we get nearer that development. :)

dreadhonk commented 4 years ago

is this ID stable across server restarts and device reconnects? if not, this would be a problem for me. if a user has multiple devices of the same make & model, I need to be able to distinguish them, even when the disconnect and reconnect and across app restarts. I want to allow the user to configure different inputs for the different outputs, and having to re-do that all the time would be kind of meh.

Depends. In C#, it's stable within a process, i.e. if you disconnect/reconnect a device, it'll keep the ID, but if the process goes down and comes back up, new ID. In JS, it's new every time. So it's kinda implementation dependent.

That said, the point you make about configuring inputs is good, and something I've gotta think about for Intiface (our UI frontend to buttplug), so this could possibly grow out of that as a config file or something.

as I said, my dream would be a hash (or hmac) with a server-unique salt or something over the hardware ID (or, if not available or unique, the physical port number (USB/serial port etc.)).

A quick overview on that since it's not covered in the spec, because it's not really part of the protocol. All of our device definitions are loaded from a JSON file. A copy of this file is compiled into the library, but can also be loaded from outside if need be. The newest unreleased version is here:

https://github.com/buttplugio/buttplug-device-config/tree/device-attributes

ah, good pointer. I was looking for this file since I considered reworking the corresponding device detection part in the current android library.

the library currently uses the BluetoothAdapter.startLeScan() method. this method does not yield any devices on my phone. I have no idea why. if I use BluetoothAdapter.startDiscovery() (+ the corresponding listeners), I get my device listed.

Oof, yeah, this is why I'm not looking forward to android support. Some of the Xamarin libraries I was using also mentioned that Android support is a very specific nightmare.

All of this would be covered in our lower level bluetooth library I think, so I'll try to keep it in mind once we start implementation there.

though note that my patches are pretty minimal actually. it's pretty trivial to support both scans side-by-side; once you got the callback from startDiscovery, you have the same objects available as with startLeScan, so I did only have to integrate the callbacks.

I mean if the library simply knows to check for permissions and fail cleanly if they're missing, that's good enough I think. though that makes it a bit tricky if an app wants to support local and remote servers, since it wouldn't need permissions for remote servers ...

The way Buttplug is built, applications just use the Client API and it should be opaque whether the server is local/remote, outside of issues like latency. So it'd just come back as a connection/scan error from the client in certain cases, which it'd be up to the app UI to handle.

so just in case you're not familiar with how the permission checking works:

  1. you call ContextCompat.checkSelfPermission which tells you whether you currently hold a permission. if you do, it's fine, you can use it. otherwise, you have to:
  2. have an activity ("=" window) which is currently active. this is equivalent to having the app's window in the foreground
  3. use that activity to call up a permission dialogue using ActivityCompat.requestPermissions. this call is asynchronous: it returns immediately.
  4. once the user has reacted to your request, a method is called on your activity (onRequestPermissionsResult) which tells you what's what. if you got the permissions, you can continue.

note that all of this happens in the same (UI) thread, so you cannot block on requestPermissions using a future or something; you would not receive the callback then. this is also handled incorrectly by the current implementation, which seems to assume that requestPermissions blocks; it simply continues assuming that it got permissions and goes crashing right away.

so especially a background-running (foreground-service, you gotta love android) server will have to delegate permission request handling to the caller/user of the library in some way and can't deal with that itself.

What part of FEARLESS GOD DAMN CONNCURRENCY do you not get? :) (It's in rust. It'll be thread safe in the rust parts).

I'm not sure I fully understand this statement.

Sorry, Rust joke. :)

Rust has specific structures in the language to maintain thread safety. Buttplug-rs uses async rust, which means we only sometimes know what thread we're on, but we're guaranteed by the language that accesses will be safe. I'll be writing up more on the Rust architecture soon, but it'll require some background in the language to understand it.

I did some rust in the past, but not much. enough to make sense of what you said now. I get that rust is quite amazing regarding thread-safety (and memory-safety, too). however, we don't have that luxury in the kotlin/java layer, so some care will have to be taken in the interface between those.

I trust that you folks can do sane API design. I don't worry about that, and if I have to change things on my end, that's fine.

Yeah, but we'll still need input on the Android side, as I'm sure the platform has idioms at the app level that we may or may not have to deal with, so I'm definitely open to input as we get nearer that development. :)

note that I'm not a very proficient android developer either, soo ... I probably can't help with android specifics (except for the bugs/weirdnesses I already found on my device).

qdot commented 4 years ago

as I said, my dream would be a hash (or hmac) with a server-unique salt or something over the hardware ID (or, if not available or unique, the physical port number (USB/serial port etc.)).

That's what WebBluetooth does, and it's something I can look into. You'll still get it as a u32 tho, 'cause that's how we index devices to the client and I can't easily change that, but it'll be the same u32 every time at least.

ah, good pointer. I was looking for this file since I considered reworking the corresponding device detection part in the current android library.

Yup, that's our main source of truth. The version I linked there will be released with the new version of the rust library, hopefully sometime this month.

though note that my patches are pretty minimal actually. it's pretty trivial to support both scans side-by-side; once you got the callback from startDiscovery, you have the same objects available as with startLeScan, so I did only have to integrate the callbacks.

Ok, that's something I can look at within the bluetooth library level then, which exists outside of buttplug. I'll file something on https://github.com/deviceplug/btleplug about it to remind myself.

so just in case you're not familiar with how the permission checking works:

Ok, so it sounds like this is something that would need to be handled at the level of an application that's integrating buttplug. We might be able to provide a helper function, but the app itself would need to get the permission, then hopefully everything we call after that would "just work".

I did some rust in the past, but not much. enough to make sense of what you said now. I get that rust is quite amazing regarding thread-safety (and memory-safety, too). however, we don't have that luxury in the kotlin/java layer, so some care will have to be taken in the interface between those.

Yeah, gonna be the same with all the FFIs. I'm planning on writing a rust book (fancy term for some nicely formatted markdown files) to cover the buttplug-rs architecture that will go over methods for abstracting this on difference concurrency models.