search

chipweinberger / flutter_blue_plus

Flutter plugin for connecting and communicationg with Bluetooth Low Energy devices, on Android, iOS, macOS
Other
726 stars 442 forks source link

L2cap merge #946

Closed rehlma closed 1 month ago

rehlma commented 1 month ago

Update and merge L2Cap changes from https://github.com/continental/flutter_blue_plus/tree/master

See L2Cap PR

chipweinberger commented 1 month ago

"57 files changed"

this will never be merged. It is too big.

I am the only maintainer of this package, and I can't maintain this code.

also, see my original comment on that previous PR

I'm open to adding this. but, it does not follow FBP conventions.

  • All java code should be in a single file
  • All iOS code should be in a single file & in obj-c
  • all messages should start with Bm and be in the bm_messages.dart file
  • messages should not be their own java or obj-c classes. follow the existing FBP code
  • no need for "MarshallingUtil" or the logging changes. that should be in a different PR
  • case L2CapMethodNames.CONNECT_TO_L2CAP_CHANNEL should just be "connectToL2CapChannel"
  • you should not add any dependencies to the example app

basically, you're not following FBP conventions at all, and you are refactoring too much code.

please open a new PR if you'd like to merge it =)

chipweinberger commented 1 month ago

I suggest you publish this as a separate package

"fbp_l2cap" or something

rehlma commented 1 month ago

It wasn't my plan to open this PR, at least not in this state. I wanted to open the PR on my fork, but this repo was preselected. There were a few features missing.

I've read your comments and will try to move everything into one file. Since the actual code changes aren't that much, it's not a big deal to move the Java or Swift code. My problem is obj-c.

But you're right, although I would prefer to merge it at some point, we could also release a separate package.