kdschlosser / samsungctl

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

Token not getting written #51

Closed mikelitka closed 5 years ago

mikelitka commented 5 years ago

I am running the following command as root:

./samsungctl -i -vvv --host 10.99.1.214 --method websocket --port 8002 KEY_VOLUP

Each time I run it I am prompted by the TV to allow the device. For some reason the token isn't being written down to the file system at ~/.samsungctl/token.txt.

Any ideas on additional troubleshooting here?

mikelitka commented 5 years ago

If I call out the token file explicitly, accept the prompt, it still doesn't get written out to file system.

[root@homeassistant bin]# ./samsungctl -vvv --host 10.99.1.214 --method websocket --port 8002 KEY_VOLUP --token ~/.samsungctl/token2.txt using saved token: /root/.samsungctl/token2.txt incoming message: {"data":{"clients":[{"attributes":{"name":"c2Ftc3VuZ2N0bA==","token":"/root/.samsungctl/token2.txt"},"connectTime":1548250672236,"deviceName":"c2Ftc3VuZ2N0bA==","id":"ea428712-5e1f-46b2-8e42-2016c3dfdc70","isHost":false}],"id":"ea428712-5e1f-46b2-8e42-2016c3dfdc70","token":"10456855"},"event":"ms.channel.connect"}

new token: 10456855 Access granted. Sending control command: {'Cmd': 'Click', 'DataOfCmd': 'KEY_VOLUP', 'Option': 'false', 'TypeOfRemote': 'SendRemoteKey'} Websocket closed [root@homeassistant bin]# ls -al ~/.samsungctl total 0 drwxr-xr-x 2 root root 6 Jan 23 08:37 . dr-xr-x---. 8 root root 288 Jan 21 19:28 .. [root@homeassistant bin]#

matthieut59 commented 5 years ago

I have submitted a PR to fix one of the issue (linked to writting file isntead of reading it), but I still get confused with the load operation trying to extract a json format whereas config was stored as a dict.

I ll let owner of the repo touch this one

here : https://github.com/kdschlosser/samsungctl/blob/master/samsungctl/config.py#L123

kdschlosser commented 5 years ago

OK so here goes. the original design of the config files is a json object. which is a python dictionary with some slight differences. This can be confusing for python "newbies" and to be honest adds a ton of additional punctuation and syntax related things that can open the door for errors. There is not enough information that is being stored that warrants using a json structure. so I opted to simply use a flat file. with

name = samsungctl
description = some description
host = 192.168.1.1
port = 55000
id = 123456789
method = websocket
timeout = 0
token = some_token_data
device_id = 4984487
upnp_locations = None

I have to make some changes to better handle parsing a loaded file But essentially what you will end up with is what you see above.

the same as above as json would look like this

{
    "name": "samsungctl",
    "description": "some description",
    "host": "192.168.1.1",
    "port": 55000,
    "id": 123456789,
    "method": "websocket",
    "timeout": 0,
    "token": "some_token_data",
    "device_id": 4984487,
    "upnp_locations": null
}

if there is so much as a missing quote or a space that is added that should not bet there parsing this will fail.

if you create a config file that looks like this.

TV name
Name = samsungctl
DESCRIPTION= some description
host=192.168.1.1
PoRt = 55000
just some random text
id = 123456789

method = websocket
timeout = 0
token = some_token_data
device_id = null

upnp_locations = None

It will read the data properly. the only thing the parser cares about seeing is the equals in the line. this is the marker that the line should be read as a config line.

Nice thing is that if you do have any lines that are not config lines. when you save the config file. it is not going to overwrite those lines.

the parsing of the json is for backwards compatibility with the old config file. once you have loaded and then saved the file the json will be removed and the new flat file will be put into it's place.

Right now I am going to make some changes to how the file gets parsed.

mikelitka commented 5 years ago

Just so I am clear, is this token not being written to the file system a configuration file issue? I am using all switches, I wouldn't have thought that the config file would be used with the switches overriding it.

kdschlosser commented 5 years ago

OK so i updated the Config parsing.

TV name
Name = samsungctl
DESCRIPTION= some description
        host=192.168.1.1
PoRt = 55000
just some random text
id = 123456789
some random key = this is fine
method = websocket
timeout = null
token = some_token_data
device_id = 

upnp_locations = None

it will now handle a file that looks like the above. I don't know why someone's config file would look like this. But one thing I have realized over the 3 years I have been programming is this. DON'T ASK, just make it so that it will handle it. LOL.

kdschlosser commented 5 years ago

You have to save the config file.

did you read the documentation???

kdschlosser commented 5 years ago

writing files to the filesystem without the user having knowledge of it taking place (other then a temp file) is not something I am very keen on. hence the reason why i did away with the token.txt file.. the user should be fully aware of what is being written to their disk. and they are the ones that should specifically ask for the data to be written. or loaded for that matter.

kdschlosser commented 5 years ago

If you are using the command line to use samsungctl I am going to add a concession to handle the config file loading and saving right now.. I just have not gotten there quite yet. one step at a time.

mikelitka commented 5 years ago

I don't need the token switch feature, I just thought they were overrides. I am happy to use the config file if that is the requirement. I did see that in the docs but again thought I could override. Thanks for your hard work here.

kdschlosser commented 5 years ago

the token is now stored in the same file as the rest of the config data for the TV. this makes it better in 2 ways. 1. I am not writing information to the users disk without them expressly telling me to do so. and 2. any token data that iis saved is unique to that TV so if something has to be written why not write all of the information pertaining to that TV and only have to pass a path to the file in the command line.

I already update the library. this is how it works

samsungctl --method websocket --host 192.168.1.1 --config-file "/path/to/configfile.config" KEY_MENU

You will only have to do that a single time. the first time you use the library on the TV after that your command line will read as follows.

samsungctl --config-file "/path/to/configfile.config" KEY_MENU
kdschlosser commented 5 years ago

if you do not specify a config file to save and load from and your TV requires the use of a token. you will have to pair each and every time.

Now you may think that is a bad thing. But it's not. before the token was stored along with the IP of the TV. so if you wanted to pair the TV to the library again you would have to manually edit the file and remove the line. Now all you have to do is specify a new file to read and write from. plus as an added bonus. if you wanted to say control the TV from your laptop without pairing the TV again. simply copy that config file to the other PC and away you go.

mikelitka commented 5 years ago

Getting this error:

[root@homeassistant samsungctl]# /common/apps/homeassistant/bin/samsungctl --method=websocket --host=10.99.1.248 --config-file ~/.config/samsungctl.conf KEY_MENU Traceback (most recent call last): File "/common/apps/homeassistant/bin/samsungctl", line 11, in load_entry_point('samsungctl==0.8.0b0', 'console_scripts', 'samsungctl')() File "/common/apps/homeassistant/lib/python3.6/site-packages/samsungctl-0.8.0b0-py3.6.egg/samsungctl/main.py", line 257, in main config = Config.load(args.config_file) File "/common/apps/homeassistant/lib/python3.6/site-packages/samsungctl-0.8.0b0-py3.6.egg/samsungctl/config.py", line 166, in load self = cls.new(**config) TypeError: object.new(): not enough arguments [root@homeassistant samsungctl]# [root@homeassistant samsungctl]# [root@homeassistant samsungctl]# [root@homeassistant samsungctl]# cat ~/.config/samsungctl.conf TV name Name = samsungctl DESCRIPTION= some description host=10.99.1.248 PoRt = 8002 just some random text id = 123456789

method = websocket timeout = 0 token = some_token_data device_id = null

upnp_locations = None

kdschlosser commented 5 years ago

ooops.. heh forgot to add cls to that bugger.

kdschlosser commented 5 years ago

ok it should be fixed now.

mikelitka commented 5 years ago

Looks like I am getting a different error now:

[root@homeassistant samsungctl]# /common/apps/homeassistant/bin/samsungctl --method=websocket --host=10.99.1.252 --config-file ~/.config/samsungctl.conf KEY_MENU Traceback (most recent call last): File "/common/apps/homeassistant/bin/samsungctl", line 11, in load_entry_point('samsungctl==0.8.0b0', 'console_scripts', 'samsungctl')() File "/common/apps/homeassistant/lib/python3.6/site-packages/samsungctl-0.8.0b0-py3.6.egg/samsungctl/main.py", line 269, in main if not config.host: AttributeError: 'NoneType' object has no attribute 'host'

[root@homeassistant samsungctl]# cat ~/.config/samsungctl.conf TV name Name = samsungctl DESCRIPTION= some description host=10.99.1.248 PoRt = 8002 just some random text id = 123456789

method = websocket timeout = 0 token = some_token_data device_id = null

upnp_locations = None

tya commented 5 years ago

@mikelitka wanna test out my fix for this issue? https://github.com/kdschlosser/samsungctl/pull/53

mikelitka commented 5 years ago

@tya Bango! Nice work that fixed it. Token is now working on repeated calls. Thanks all for getting this sorted!