[androiddebugbridge] Missing part to refresh hdmi #48
Conversation
Signed-off-by: Tom Blum <trinitus01@googlemail.com>
Signed-off-by: Tom Blum <trinitus01@googlemail.com>
| @@ -117,9 +121,18 @@ public void sendText(String text) | |||
| runAdbShell("input", "text", URLEncoder.encode(text, StandardCharsets.UTF_8)); | |||
| } | |||
|
|
|||
There was a problem hiding this comment.
I have addressed this in to a separate pr, so you don't need to worry about it. I've also added a regex for the packageName to avoid passing weird strings to the command.
| @@ -170,12 +183,14 @@ public String getCurrentPackage() throws AndroidDebugBridgeDeviceException, Inte | |||
|
|
|||
| public Optional<Boolean> isHDMIOn() throws InterruptedException, AndroidDebugBridgeDeviceException, | |||
| AndroidDebugBridgeDeviceReadException, TimeoutException, ExecutionException { | |||
| Boolean fallbackHDMI = fallbacks.getOrDefault(HDMI_STATE_CHANNEL, false); | |||
| String result = runAdbShell("cat", "/sys/devices/virtual/switch/hdmi/state"); | |||
There was a problem hiding this comment.
You are not avoiding running the command once the fallback is enabled. I'll move the logcat command into a separate private method and call it directly once fallback enabled becomes true.
| return Optional.of(result.equals("1")); | ||
| } else if (result.isEmpty()) { | ||
| } else if (fallbackHDMI == false && result.isEmpty()) { | ||
| return Optional.empty(); | ||
| } else { |
There was a problem hiding this comment.
I think that we should look for a concrete output in the command to enable the fallback, something like an error output that said file is not there.
There was a problem hiding this comment.
The result empty check is not for the fallback, it is to keep the state if nothing returns. If something returns it must be a valid info or an error. If it is empty the log hasnt the value anymore and it waits for a new state. So it is really necessary to check for empty.
There was a problem hiding this comment.
That's right, my bad.
We can also have thrown a AndroidDebugBridgeDeviceReadException as for other inconclusive results. We can let it as is if you prefer but I think it will be more accurate because in this way the command can fail forever and the user is not informed. What do you think?
There was a problem hiding this comment.
Maybe i have to describe a little bit more. Lets say the logcat show up max 30 Lines in a loop. Now the last lines shows the status "hdmi on". After this the hdmi stays on for lets say 30 minutes. But after 10 minutes the logcat doesnt show the status "hdmi on" anymore because of the log rotation. Why to inform the user every 30 seconds for the timespan of 20 minutes that the binding has no new state. It is not an error or something. After 30 minutes the user turn off the tv and the logcat shows hmdi off, now the state is off, and so on. For me there is no point the state becomes out of sync or stays in a wrong state. Maybe i missunderstood your request? Maybe because i transfered this behavior also to the other commands without logcat or log rotation? This isnt necessary like it is for the logcat but i thought if a command on a shell will not return any output it is also not an error because else it would return something. So on a closer look this is only necessary for the result of the logcat commands.
There was a problem hiding this comment.
But i slowly understand whats your point. For the logcat part its a big problem to know if there is no status at the moment or if there never will be a status and nobody knows. Maybe we can inform the user there is no new status but not that often or not on every refresh? Still thinking about a solution.
There was a problem hiding this comment.
I think you could check for "No such file or directory" or "Permission denied" in the output in order to enable the fallback.
There was a problem hiding this comment.
If you only need the value as a state, why not directly return State instead of the Optional<Boolean> and ude UndefType.NULL if there is no value. You could check for that before posting the update. You could also make a difference with UndefType.UNDEF if you really don't know.
There was a problem hiding this comment.
I'm talking about the return of the cat command you use at first not logcat command. What you said applies to the output of the logcat command but I don't think it's the same for the cat output.
Anyway I can't test that on my device:mantis:/ $ cat /sys/devices/virtual/switch/hdmi/state /system/bin/sh: cat: /sys/devices/virtual/switch/hdmi/state: Permission denied
Yes you are right. The empty check is not necessary for other checks than logcat. But, i also think it is not necessary to check for the string "permission denied" or other results because if the result does not return 0 or 1 after the split and if the fallback won't returns "SWITCH_STATE=" it should throw a AndroidDebugBridgeDeviceReadException. So lets check for the common returns and if not we throw the exception?
There was a problem hiding this comment.
For example: If /sys/devices/virtual/switch/hdmi/state does not return any of the expected results (or the fallbacks), throw an exception because it seems the device does not support the command.
There was a problem hiding this comment.
removed all the fallback remember suff
Signed-off-by: Tom Blum <trinitus01@googlemail.com>
* added: missing part to refresh hdmi Signed-off-by: Tom Blum <trinitus01@googlemail.com>
added: missing part to refresh hdmi
Signed-off-by: Tom Blum trinitus01@googlemail.com