search

Lakritzator / harmony

C# library for connecting to and controlling the Logitech Harmony Hub.
MIT License
5 stars 0 forks source link

Sending command not working #4

Closed Slion closed 8 years ago

Slion commented 8 years ago

So I just tried the latest changes. The exception is gone but commands are not working. Did double check that @hdurdle implementation is working fine.

Slion commented 8 years ago

Looks like we send the same IQ as @hdurdle Yet something goes wrong, could be a problem with our XMPP.

Lakritzator commented 8 years ago

I also outputted the messages, to compare them, and I didn't notice a difference. BUT how do you test, I can't get it work with the original either... ?

Slion commented 8 years ago

With a TV and a Hub. First get the device ID:

HarmonyConsole.exe  -i HarmonyHub -u <e-mail> -p <password> -l d

Reusing token: ab0d59f2-599b-4a9b-0418-9bd7498ad514
Devices:
Bathroom : DigitalMusicServer (37058231)
HP Media Center : MediaCenterPC (37058138)
Kitchen : MediaCenterPC (37058232)
Kitchen (2) : DigitalMusicServer (37058233)
MCE Keyboard : MediaCenterPC (37058141)
Philips 4K TV : Television (37058142)
Philips TV : Television (37058140)
SONOS : Amplifier (37058139)

Then find a command you want to run:

HarmonyConsole.exe  -i HarmonyHub -u <e-mail> -p <password> -d 37058140

Reusing token: ab0d59f2-599b-4a9b-0418-9bd7498ad514
PowerToggle (Power Toggle)
Number0 (0)
Number1 (1)
Number2 (2)
Number3 (3)
Number4 (4)
Number5 (5)
Number6 (6)
Number7 (7)
Number8 (8)
Number9 (9)
Mute (Mute)
VolumeDown (Volume Down)
VolumeUp (Volume Up)
PrevChannel (Prev Channel)
ChannelDown (Channel Down)
ChannelUp (Channel Up)
DirectionDown (Direction Down)
DirectionLeft (Direction Left)
DirectionRight (Direction Right)
DirectionUp (Direction Up)
Select (Select)
Stop (Stop)
Play (Play)
Rewind (Rewind)
Pause (Pause)
FastForward (Fast Forward)
Record (Record)
Menu (Menu)
Subtitle (Subtitle)
Teletext (Teletext)
List (List)
Green (Green)
Red (Red)
Blue (Blue)
Yellow (Yellow)
Info (Info)
Exit (Exit)
Aspect (Aspect)
2D (2D)
3D (3D)
Adjust (Adjust)
Ambilight (Ambilight)
Find (Find)
InputExt1 (InputExt1)
InputExt2 (InputExt2)
InputExt3 (InputExt3)
InputHdmi1 (InputHdmi1)
InputHdmi2 (InputHdmi2)
InputHdmi3 (InputHdmi3)
InputHdmiSide (InputHdmiSide)
InputSat (InputSat)
InputSide (InputSide)
InputTv (InputTv)
InputVga (InputVga)
NetTv (NetTv)
Options (Options)
Source (Source)

Then run it, here sending power toggle to my TV:

HarmonyConsole.exe -i HarmonyHub -u <e-mail> -p <password> -d 37058140 -c PowerToggle

Slion commented 8 years ago

Could we easily swap back to that other XMPP library? Why did we go for this one over the other in the first place?

Slion commented 8 years ago

I think I found the problem. It's a character case issue apparently. Basically we have to get rid of JavaScriptSerializer and replace them by DataContractJsonSerializer. JavaScriptSerializer just does not use data contract.

Lakritzator commented 8 years ago

That was one of the things I was worried about, well not directly, but about the case in the json.

You were a bit ahead of me, I was still struggling with getting a reliable test. I actually can't get any version working reliable, not the original either... it does work, sometimes.

On a side-note, the device ID's change with every session... nice for testing :disappointed:

I will make a fix, or are you already on it?

Lakritzator commented 8 years ago

I now saw it, "deviceId" vs "DeviceId"

Lakritzator commented 8 years ago

I already made a fix, which could work, but I noticed a different "issue" which I need to check. Explain later... But my dog is getting bored, I need to take her out a bit. Will push the json fix first in a few minutes.

Lakritzator commented 8 years ago

Pushed Oh I added a readline to the program at the end, you might want to remove this.

Lakritzator commented 8 years ago

The other issue is that there is not response coming from the harmony which can be correlated, maybe this is not needed but the client "hangs" waiting for the response... I will check the details later.

The reason for the XMPP change just came to me, it was the fact that the other didn't support hostnames.

Slion commented 8 years ago

You could have waited 30mn until I pushed my very similar changes mate. We are wasting both our time working on the same stuff.

Slion commented 8 years ago

My changes are there: https://github.com/Slion/harmony/commit/1cebf77b0d68688ed447aa6ef7bc893be515db40

Lakritzator commented 8 years ago

We only waste a little, it's good we both understand the issues.

I have copied some small modifications from your code, but rather have my changes as I already started to introduce better error handling. e.g. my local version handles the errors better, it would say what went wrong if you send the false device ID. Still have stuff to do, maybe it's better if you wait a bit with changing stuff?

Removing the await is wrong too, as that would dispose the client to quickly, and besides the login isn't awaited and in that case the client might send commands before the connection is property build.

I think there is a small error in the XMPP library, as this tells me (caught exception) something about partial content or something similar, maybe this is the reason some requests return empty. I need to see if I can track the data which is send over the wire.

Slion commented 8 years ago

Just let me now how you want to proceed. Please get it in a state that's usable ASAP so that we can release it on NuGet and consume it. Major rework and improvements can wait for later release.

Lakritzator commented 8 years ago

I found some additional information, quite important... Sending a command with "status=press", which is done right now, will afaik have the result that when someone sends "Volume Up" has a party... There is no release, so the volume will be upped to the max :grin:

I already implemented a small fix for that, but in fact I think we should collect a bit more information as there still are a few questions which I would like to have answered before I blow up someones speakers!

The most important thing right now is to understand how commands are "confirmed" (even if), I know that an error can occur (errorcode="565" device unknown) and I know we get an empty message (but I am not sure it is supposed to be empty or even due to the command send.

And we need to add timestamps to the command, I found something about this here: https://github.com/mhop/fhem-mirror/blob/master/fhem/FHEM/37_harmony.pm They send an press with timestamp 0, and a release with timestamp 100, which should mimic a press/release on the remote. The API should support press & hold & release and press & release, don't you think?

Leter we should add activity changes, this is lower priority but I assume that if the client connection stays open we also get status updates, which we can promote as events. Should be quite easy...

Maybe there is some more information in the FHEM harmony implementation.

Where will we run the build & nuget packaging? (We don't run it on a PC, due to virus etc, I normally use AppVeyor)

Lakritzator commented 8 years ago

I didn't push changes yet, I need to go shopping and stuff... later tonight after a BBQ party (not so late) I 'll have some time to finish my changes.

Lakritzator commented 8 years ago

I just found why I sometimes couldn't test, I was sending to the Harmony in the bedroom instead of my living room where I sit on the sofa :grinning:

Lakritzator commented 8 years ago

So, I pushed my changes (I'm not going to the BBQ). These should make the SendCommandAsync work reliable, also detect errors. I have made a SendKeyPressAsync, which handles the "press & release" for you.

Next on the agenda, is testing activity changes. And after that: #5

If that works, I want to go over the credentials part to see how far I can enhance it that it works behind a proxy without adding dependencies... Or maybe I just make some very basic information available so one can add this to his application however they like. (and use Dapplo :grin: )

Than we need to see if we can get Howard to look over the changes and accept the PR. Also we need to decide on who supplies the build-server for the nuget packages.

I will create tickets for the tasks.

P.S. The error handling is probably not yet perfect, I'm not sure about having exceptions everywhere. But for now it's acceptable...

Lakritzator commented 8 years ago

One thing I just remembered, I want to make sure the API doesn't have a differentiation between long and short presses. As it doesn't make sense... I don't think this is the case, as we send directly to the device, but just in case?

Slion commented 8 years ago

I'm not going to the BBQ

You should mate, the weather is really nice.

I have made a SendKeyPressAsync, which handles the "press & release" for you.

I'm not convinced this was needed. Did you check if the key press on it's own was not just fine? Could you reproduce that potential volume bug you mentioned earlier?

One thing I just remembered, I want to make sure the API doesn't have a differentiation between long and short presses. As it doesn't make sense... I don't think this is the case, as we send directly to the device, but just in case?

I believe it's not like your remote. You are just executing a function on the device not sending remote key presses.

Slion commented 8 years ago

Also we need to decide on who supplies the build-server for the nuget packages.

It looks like you want to take ownership of the whole thing so feel free to do it all. I'm just fine with that.

Lakritzator commented 8 years ago

Nope, not really, I was hoping Howard would :)

Lakritzator commented 8 years ago

About the timestamp stuff, I read that somewhere and didn't make it up... I will test this, but with volume down of course! I am still busy with the Auth code (HttpClient).

And no, I didn't want to go to the BBQ, although the weather is great, there are 20 people who smoke and I don't.

Lakritzator commented 8 years ago

I believe it's not like your remote. You are just executing a function on the device not sending remote key presses.

Well I am confused, it actually doesn't do what I understood from different discussions I found. If I send a volume down, it just does that... what is this press & release and timestamp stuff than good for.

Lakritzator commented 8 years ago

Ah, see here: https://github.com/jterrace/pyharmony/blob/master/PROTOCOL.md

Slion commented 8 years ago

While the send command appears to work when sending a single command it looks like the press and release scheme is needed if you are sending multiple commands one after the others.

Lakritzator commented 8 years ago

Ah, this is good to know, so the single press is actually rather unneeded.

Slion commented 8 years ago

Indeed, I guess it could be removed at some point. I've left it in for now as it could still be useful while I'm doing a lot of testing. Basically both methods work but the the Key Press approach typically delivers faster response time depending on how it takes to get the server acknowledgement.

See: https://github.com/Slion/harmony/issues/11