kdschlosser / samsungctl

Remote control Samsung televisions via a TCP/IP connection
MIT License
154 stars 33 forks source link

encrypted method error #84

Closed p3g4asus closed 5 years ago

p3g4asus commented 5 years ago

I am sorry @kdschlosser. I could test the new commits on the encrypted method only now. I have found some bugs and solved them. I am attaching the patch file (renamed into txt because otherwise github will not accept it). Furthermore, to make the things work, when using encrypted method and no config file the port should not be entered in the command line. Otherwise app_id and device_id fields of config object won't be set and pairing process won't work (it will complain for wrong PIN). remote_encrypted.txt

kdschlosser commented 5 years ago

I do not understand what exactly the fix is... I looked at the patch.. and all you did was double up an if state looking for the same thing.

if "secure-mode" in response.content:
    js = response.json()
    if "secure-mode" in js:
        raise RuntimeError(
            "TODO: Implement handling of encryption flag!!!!"
        )

that is the exact same thing as

if "secure-mode" in response.content:
    raise RuntimeError(
        "TODO: Implement handling of encryption flag!!!!"
    )
p3g4asus commented 5 years ago

You are not right. The new source after applying the patch will be:

js = response.json()
if "secure-mode" in js:
        raise RuntimeError(
            "TODO: Implement handling of encryption flag!!!!"
        )

The previous code raises an exception because response.content is a byte object and you cannot look for a string inside a byte object (you can do it in python 2 where bytes==str not in python 3). This way you look for a key in a dict.

kdschlosser commented 5 years ago

I fixed the byte issue. I will be pushing the new code shortly.

kdschlosser commented 5 years ago

I am not sure where in the JSON structure the "secure-mode" is located. I have not seen anyone that has posted an issue about that exception. so instead of me guessing that the "secure-mode" element is actually at the base of the structure. It is best to leave it how it is doing the simple string search. I did realize from looking at the code the byte problem. and I did fix that.

So until I know for sure where in the JSON structure the "secure-mode" is I am not going convert the returned data. what you are doing is the correct way. I will not deny that. and that is the way I would have done it as well. But with the very large ? and the possibility of breaking the code (further then I have done already LOL), I am opting for not causing further complications. I think you can agree with that.

also I have not messed around with patch files to much. I missed the - sign. sorry about that. I had already fixed all of the decodes added to the logging statements. I forgot to push it last night. sorry about

also... I just went back and double checked.

third_step_response: {"auth_data":"{\"auth_type\":\"SPC\",\"request_id\":\"1\",\"ClientAckMsg\":\"0104000000000000000014CEA0857A91E9B9511CC1453433CE79BE222FF32A0000000000\",\"session_id\":\"1\"}"}

you are correct with the loading of ['auth_data'] with json. I missed that. TY for finding that. I would rather you do a PR for that fix if possible and if not I can add your name to the commit message, However you want to go about it. I just like to make sure that the people that contribute get the credit for it. I know it's only github and all. But it is still the right thing to do.

kdschlosser commented 5 years ago

also... since you seem to have a handle on python... would you be willing to take on a project for me? I do not own an encrypted TV so I would not have the ability to test, and it would be way to much back and forth to get it all working correctly..

Right now the remote_encrypted portions (except for the __init__) are not python 2 compatible. I would very much like to make them compatible.

If this is something you would be willing to do that would be awesome! Just let me know.

p3g4asus commented 5 years ago

I have made the whole encrypted part python 2 compatible. Thare are several fixes to do. Let me make some more checks. I will post the patch here. Actually my code seems to work in python 2 and 3 just fine.

p3g4asus commented 5 years ago

Additional note: all the changes in the previous patch I posted where fully and extensively tested.

kdschlosser commented 5 years ago

so you just want me to add your name to the commit message?? or do you want to do a PR for the changes?

I have not messed around with patch files. I do not even know how to go about applying them. I know how to read them with my eyes. other then that they are beyond me LOL.

p3g4asus commented 5 years ago

OK let's do a PR. I do not know exactly what a PR is. Please tell me what to do. Should I simplly read your code and test it?

kdschlosser commented 5 years ago

PR = Pull request a PR a request to add changes that you made into my code. It is going to let you know if there are conflicts between my code and your code. It provides a history of what changes were made and when. and it also give the ability to reverse the changes in the even it causes some kind of a failure. Because as you have already probably discovered that if you make a bunch of changes and those changes cause the program to fail trying to remember those changes is not always easy to do. since the dawn of the idea of a "template" is when version control was first invented. which is what GitHub is. so when you want to modify something but are unsure iif the modifications will work.. you will make a copy of the original so you have something to go back to if the modification fail.

OK this is what you want to do..

Fork my repository. then go to your fork of it. it will have created a repository called samsungctl in your list of repositories be careful here because there will also be one listed as kdschlosser/samsungctl that is not the one you want.

once you are in your fork. make a new branch. you do this by click on the branch:r button. and in the text field at the top of the drop down that appears you can enter a name.

once that is done. make sure that the button now states branch:[THE NAME YOU ENTERED]

You never want to modify the master branch. you always use that master branch to create new branches and then make modifications to the new branch. this makes it easier to separate changes you make. which in turn also does not get the PR's all messed up. a PR (Pull Request) is done for the code that is in a branch.

so think about it this way if you have a single branch. we will call it branch1 and you make changes to branch1 and create a PR for those changes. then you make more changes to Branch1 and you want to make a PR for those changes. the problem is once you commit the second set of changes to the Branch it is going to alter the first PR you made... this is not what we want to do.

If the second set of changes you make are dependent on the first set of changes. then you will make a branch of Branch1 and then edit that new Branch

if they are not dependent on the changes made in Branch1 then you will make a new branch from Branch Master,

does that make sense?? If not let me know and I will try to think of a better example to give, This is probably the hardest thing to understand about GitHub. But it essentially boils down to this. once you create a PR for a code change in a branch. and changes you make and commit to that branch are going to change the code in the PR. so unless the changes are supposed to be for that PR you do not want to edit that branch. you want to make a new one to modify.

so now that you have made a new branch.. modify that branch. commit the changes to that branch.. once you are done.. then create a PR against my master branch. when start the PR creation process. you will be shown a page where you can enter a title and description of the PR. just above that is where you will be able to select where you want the PR to go. and where the pr is coming from. You want to make sure that the "go" portion of it is to my master branch. and the "from" is from the branch where you made the changes.

go ahead and mess about with it. if there is something messed up with it we can always fix it.

kdschlosser commented 5 years ago

when i accept a PR it is going to change the code to my master branch. this is going to make your master branch out of sync from my master branch in order to get them back into sync this is going to require you to have git installed on your PC. and then you will open a command prompt (shell)

these lines are to make the local copy of your fork. it is only needed to be run a single time. find a nice copy place to store the local copy. under Windows this is typically going to be

%USERPROFILE%\documents\github

you want to go to the directory.. then create a new folder called samsungctl. go into that folder. and run the following command.

 git pull samsungctl master

if it kicks out an error change master to origin.

this will clone your fork to your local machine. you will only need to do this a single time.

to get your Fork back into sync with repo you will need to be in that same samsungctl folder and run the following

git checkout master
git pull https://github.com/kdschlosser/samsungctl.git master
git push samsungctl master

again if it errors change master to origin.

this will clone my repository replacing your local one. and then it will push that new local version to giithubs servers.

kdschlosser commented 5 years ago

you can create a batch file or a shell script to automate syncing of your repo.

p3g4asus commented 5 years ago

Thank you. You where very clear. I will do a pull request for both python2 compatibility and bug fix for remote_encrypted