StephenBlackWasAlreadyTaken / xDrip-Experimental

Experimental Branches for Collaboration on DexDrip
GNU General Public License v3.0
25 stars 62 forks source link

No Broadcast Permission #364

Closed AdrianLxM closed 8 years ago

AdrianLxM commented 8 years ago

As agreed upon: No more permission.

Tested it with older NightWatch versions - works.

MilosKozak commented 8 years ago

great

AdrianLxM commented 8 years ago

I also refactored out the whole broadcast that it can be used by other apps (like the Medronic ones) to have the same kind of connectivity. Also tested on NightWatch.

MilosKozak commented 8 years ago

do you broadcast xdrip version somehow to be able to warn user if using old xdrip (after i remove permission from nsclient)?

MilosKozak commented 8 years ago

i got the idea now but maybe now would be the right time to rename/add this broadcast to something else (like info,nightscout.sgv) and keep the old as it is now at least for some time

victorkp commented 8 years ago

@MilosKozak if you leave the uses-permission in nsclient, then it should work with both old and new versions of xdrip. On Android, if you <use-permission android:name="non-existant_permission"/> then it's just a noop.

MilosKozak commented 8 years ago

well there are at least 4-5 apps (as i know) using this broadcast. i think we must remove permission everywhere to get rid of issues we have now. that's why i'm thinking about new name

tzachi-dar commented 8 years ago

Can we also pass the filtered bg value and filtered raw as well?

AdrianLxM commented 8 years ago

@MilosKozak I can add a sender-ID String. Old NightWatch versions will just ignore them.

Any Idea for that? Or Sender-ID-String and a Version-Number.

MilosKozak commented 8 years ago

i'd prefer new broadcast as close as possible to NS record

AdrianLxM commented 8 years ago

@MilosKozak I think that's a different thing. I'd really like to keep the legacy xDrip broadcast for now.

But if you want a new broadcast with new names, feel free to add/modify BgEstimateBroadcaster.java to what you need "as close as possible to NS record". I'm happy to add this to xDrip then.

MilosKozak commented 8 years ago

@AdrianLxM i know i was pushing on you to remove permission but now i see new broadcasts as better solution. it would make lot of things easier when we move something like NS API to internal communication

MilosKozak commented 8 years ago

maybe broadcast JSONs sending to NS?

AdrianLxM commented 8 years ago

I meant for (just?) xDrip I'd like to keep the legacy broadcast.

But before giving out library classes for other apps (even though it is not much): Now is the time to have a new/better broadcast format.

I actually don't mind the internal format as we should have one library for all apps. Would you want to define a format/structure that fits NS-Client as a central data hub, @MilosKozak?

I'd like to decouple this from this PR though.

MilosKozak commented 8 years ago

ok open new thread for this discussion somewhere

AdrianLxM commented 8 years ago

ah, just to add: If it stays within android, I don't see a reason to convert it to JSON (and then back on the receiving side).