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

Intent interface for MainService #98

Merged
merged 56 commits into from
Jun 13, 2023
Merged

Intent interface for MainService #98

merged 56 commits into from
Jun 13, 2023

Conversation

bk138
Copy link
Owner

@bk138 bk138 commented Aug 14, 2022

This refactors the code to enable starting the MainService from the outside via an Intent interface.

Closes #83, closes #71, closes #40, closes #50, closes #95, closes #118.

Builds upon and obsoletes #84.

@bk138
Copy link
Owner Author

bk138 commented Dec 21, 2022

challenge w/ https://github.com/bk138/droidVNC-NG/tree/feat-intent-remote-control-orig

  • onStartCommand returns START_REDELIVER_INTENT when an OK state was reached from ACTION_START
    • i.e. ACTION_HANDLE_MEDIA_PROJECTION_RESULT or ACTION_HANDLE_WRITE_STORAGE_RESULT will get redelivered
  • onStartCommand for ACTION_CONNECT needs to return something as well, but it can't be START_REDELIVER_INTENT because an ACTION_START would be needed before to get the server started

approach 1

  • ACTION_START starts, again does nothing
  • ACTION_CONNECT only does sth after ACTION_START successful
  • ACTION_STOP only stops after ACTION_START successful, otherwise does nothing

👎 introduces state into Intent interface
👍 extras clearly mapped to actions where they're actually consumed
👎 but CONNECT extras also in ACTION_START, no way to do more than 1 outbound connection w/ 1 Intent call

approach 2

  • only one ACTION_START which can be called multiple times
  • action decided on payload, i.e. extras

👎 some extras invalid, state went into extra valid/invalid
👎 with one set of CONNECT extras also in ACTION_START, no way to do more than 1 outbound connection w/ 1 Intent call

approach 3 (bit broader)

  • give ACTION_APPLY a complete (JSON?) description of what should be started/stopped
  • ACTION_QUERY for getting complete state (and modiifying and re-applying...)

👍 can init several outgoing conns at once
👍 usable for complete after-kill-reinstantiate
❗ way more complex impl w/ state management
⚠️ we would have a SET action (which could return START_REDELIVER_INTENT to get back to before-crashed) BUT would also need a GET action for cases where we want to change just one outgoing connection, for instance) -> what would the GET handling return? --> can not return START_REDELIVER_INTENT -> same problem class as with individual verbs...

@bk138 bk138 linked an issue Jun 2, 2023 that may be closed by this pull request
@bk138 bk138 closed this Jun 6, 2023
@bk138 bk138 force-pushed the feat-intent-remote-control branch from cc0a669 to 1e37abe Compare June 6, 2023 08:27
@bk138 bk138 reopened this Jun 6, 2023
@bk138
Copy link
Owner Author

bk138 commented Jun 6, 2023

intent interface, take 2

verb approach 1 ⛔

  • PRO:
    • looks nice and semantic
    • not so much internal state tracking, we just react on commands from outside
  • CON
    • does not map to internal structure, app does not work this way!! ⛔⛔

verbs w/ args

  • start

    • pw
    • scaling
    • filetransfer
    • view_only
  • listen ⛔

    • listen_port
  • connect_reverse

    • reverse_port
    • reverse_host
  • connect_repeater

    • repeater_port
    • repeater_host
    • repeater_id
  • stop

verb approach 2

  • bit like orig approach, but no connect args for start verb
  • PRO
  • CON
    • caller has to do 2 calls to connect outbound: start then connect_*
    • needs feedback on calls via broadcast

verbs w/ args

  • start - does nothing on 2nd call

    • pw
    • scaling
    • filetransfer
    • view_only
    • listen_port
  • connect_reverse - needs start before, connects to 2nd host on 2nd call

    • reverse_port
    • reverse_host
  • connect_repeater - needs start before, connects to 2nd host on 2nd call

    • repeater_port
    • repeater_host
    • repeater_id
  • stop - does nothing on 2nd call

@bk138 bk138 force-pushed the feat-intent-remote-control branch from cbe718c to 8469863 Compare June 6, 2023 10:34
@bk138 bk138 removed a link to an issue Jun 6, 2023
@bk138
Copy link
Owner Author

bk138 commented Jun 9, 2023

@bk138 bk138 force-pushed the feat-intent-remote-control branch 5 times, most recently from 362ef15 to 45b44d9 Compare June 12, 2023 07:42
@bk138 bk138 force-pushed the feat-intent-remote-control branch 2 times, most recently from c4d4bef to bd03db1 Compare June 13, 2023 10:34
bk138 added 26 commits June 13, 2023 12:34
This is a Defaults implementation detail and not to be used by other components.
They are a MainActivity implementation detail and not to be used by other components.
They are a MainService implementation detail and not to be used by other components.
@bk138 bk138 force-pushed the feat-intent-remote-control branch from bd03db1 to e76521a Compare June 13, 2023 10:35
@bk138 bk138 merged commit a48873d into master Jun 13, 2023
@bk138 bk138 deleted the feat-intent-remote-control branch June 13, 2023 10:38
@FilipStadler
Copy link

DoidVNC-NG is very cool, I have been looking for a way to control a allwiner H313/X96Q smartbox/TV because it starts mumla client connected with a real radio via a soundcard making a analog to digital solution - both mumla app and DroidVNC-NG start at boot it give a warning and I need to confirm that remote control is allowed else its cool if I add maybe openvpn to the tv box.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment