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

Mod for switchbot api 1.1 #491

Merged
merged 7 commits into from
Nov 16, 2023

Conversation

y-yosuke
Copy link
Contributor

  • modifications for SwitchBot API v1.0 and v1.1
    • for API v1.0 set only token option to switchbot.launch the same as before
    • for API v1.1 set both token and secret options to switchbot.launch
  • fix codes around infrared remotes and scenes

Copy link
Member

@mqcmd196 mqcmd196 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for your contribution!

@@ -1,11 +1,13 @@
<launch>
<arg name="token" />
<arg name="secret" default="secret_none" />
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please set the default value as the empty string, so

 <arg name="secret" default="" />

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I changed the default value for secret as the empty string.
Additionally a nested (inner?) quate is needed as below.

<arg name="secret" default="''" />

dc12bac#diff-f9fd144a93e7e845e1d875167f9c1865f75157f0ec4158b663c183fca366bee5

return sign, str(t), nonce

def make_request_header(self, token, secret):
sign,t,nonce = self.make_sign(token, secret)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sign, t, nonce = self.make_sign(token, secret)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

def __init__(self, token):
self._host_domain = "https://api.switch-bot.com/v1.0/"
def __init__(self, token, secret="secret_none"):
if secret=="secret_none":
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As I commented in launch file, this line would be

secret=""

and

if not secret:

And I request to add printing the version information like

rospy.loginfo("Using SwitchBot API v1.0")

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

switchbot.py was modified to match the empty secret with adding an instance self.api_version.
And I change the instance name host_domain back to the original name _host_domain.

    def __init__(self, token, secret=""):
        if not secret:
          self.api_version = "v1.0"
        else:
          self.api_version = "v1.1"
        self._host_domain = "https://api.switch-bot.com/" + self.api_version + "/"

dc12bac#diff-396497c868baf8630a5e57ccf98059c7d6f1d45e91faccfe0917e1012f124191R20-R25


The method print_baseurl() was changed to print_apiversion() and fixed the unmatched name of the internal variables.

    def print_apiversion(self):
        if self.bots is None:
            return
        
        apiversion_str = 'Using SwitchBot API ';
        apiversion_str += self.bots.api_version;
        rospy.loginfo(apiversion_str)        

dc12bac#diff-1690cbebb7bf92ad5c7be4524a54f4c0566916a4da9a6b0c493666e891db80c9R72-R78

0b1a802#diff-1690cbebb7bf92ad5c7be4524a54f4c0566916a4da9a6b0c493666e891db80c9R76-R78

Copy link
Member

@mqcmd196 mqcmd196 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@@ -1,11 +1,13 @@
<launch>
<arg name="token" />
<arg name="secret" default="''" />
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you can use null and it will be treated as None in python.

Suggested change
<arg name="secret" default="''" />
<arg name="secret" default="null"/>

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In my environments, both <arg name="secret" default="null" /> and <arg name="secret" default="" /> make an error load_parameters: unable to set parameters (last param was [/switchbot_ros/secret=None]): cannot marshal None unless allow_none is enabled and quit the process. But <arg name="secret" default="''" /> works finely.

  • My Environments
    • Ubuntu 20.04.6 LTS - ROS Noetic - Python 3.8.1
    • Ubuntu 18.04.6 LTS - ROS Melodic - Python 2.7.17

Does this problem depend on just only my environments?
Or do you know any solutions for the load_parameter error other than <arg name="secret" default="''" />?


The console output with load_parameter error occurs when I used <arg name="secret" default="null" /> and <arg name="secret" default="" /> in switchbot.launch is shown as followings.

robotuser@robotuser-PC:~/switchbot_ws$ roslaunch switchbot_ros switchbot.launch token:='(省略)' --screen
... logging to /home/robotuser/.ros/log/fc70881c-34c9-11ee-84c2-43599b20a8be/roslaunch-robotuser-PC-68523.log
Checking log directory for disk usage. This may take a while.
Press Ctrl-C to interrupt
Done checking log file disk usage. Usage is <1GB.

started roslaunch server http://robotuser-PC:34991/

SUMMARY
========

PARAMETERS
 * /rosdistro: noetic
 * /rosversion: 1.16.0
 * /switchbot_ros/secret: None
 * /switchbot_ros/token: ca6f439a05b820c78...

NODES
  /
    switchbot_ros (switchbot_ros/switchbot_ros_server.py)

auto-starting new master
process[master]: started with pid [68531]
ROS_MASTER_URI=http://localhost:11311

setting /run_id to fc70881c-34c9-11ee-84c2-43599b20a8be
process[rosout-1]: started with pid [68541]
started core service [/rosout]
load_parameters: unable to set parameters (last param was [/switchbot_ros/secret=None]): cannot marshal None unless allow_none is enabled
Traceback (most recent call last):
  File "/opt/ros/noetic/lib/python3/dist-packages/roslaunch/__init__.py", line 347, in main
    p.start()
  File "/opt/ros/noetic/lib/python3/dist-packages/roslaunch/parent.py", line 316, in start
    self.runner.launch()
  File "/opt/ros/noetic/lib/python3/dist-packages/roslaunch/launch.py", line 676, in launch
    self._setup()
  File "/opt/ros/noetic/lib/python3/dist-packages/roslaunch/launch.py", line 663, in _setup
    self._load_parameters()
  File "/opt/ros/noetic/lib/python3/dist-packages/roslaunch/launch.py", line 354, in _load_parameters
    r  = param_server_multi()
  File "/usr/lib/python3.8/xmlrpc/client.py", line 879, in __call__
    return MultiCallIterator(self.__server.system.multicall(marshalled_list))
  File "/usr/lib/python3.8/xmlrpc/client.py", line 1109, in __call__
    return self.__send(self.__name, args)
  File "/usr/lib/python3.8/xmlrpc/client.py", line 1447, in __request
    request = dumps(params, methodname, encoding=self.__encoding,
  File "/usr/lib/python3.8/xmlrpc/client.py", line 968, in dumps
    data = m.dumps(params)
  File "/usr/lib/python3.8/xmlrpc/client.py", line 501, in dumps
    dump(v, write)
  File "/usr/lib/python3.8/xmlrpc/client.py", line 523, in __dump
    f(self, value, write)
  File "/usr/lib/python3.8/xmlrpc/client.py", line 576, in dump_array
    dump(v, write)
  File "/usr/lib/python3.8/xmlrpc/client.py", line 523, in __dump
    f(self, value, write)
  File "/usr/lib/python3.8/xmlrpc/client.py", line 594, in dump_struct
    dump(v, write)
  File "/usr/lib/python3.8/xmlrpc/client.py", line 523, in __dump
    f(self, value, write)
  File "/usr/lib/python3.8/xmlrpc/client.py", line 576, in dump_array
    dump(v, write)
  File "/usr/lib/python3.8/xmlrpc/client.py", line 523, in __dump
    f(self, value, write)
  File "/usr/lib/python3.8/xmlrpc/client.py", line 527, in dump_nil
    raise TypeError("cannot marshal None unless allow_none is enabled")
TypeError: cannot marshal None unless allow_none is enabled
[rosout-1] killing on exit
[master] killing on exit
robotuser@robotuser-PC:~/switchbot_ws$ ^C

@@ -26,9 +26,20 @@ def __init__(self):
self.token = f.read().replace('\n', '')
else:
self.token = token

# Switchbot API v1.1 needs secret key
secret = rospy.get_param('~secret')
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
secret = rospy.get_param('~secret')
secret = rospy.get_param('~secret', None)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Comment on lines 32 to 36
if os.path.exists(secret):
with open(secret) as f:
self.secret = f.read().replace('\n', '')
else:
self.secret = secret
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
if os.path.exists(secret):
with open(secret) as f:
self.secret = f.read().replace('\n', '')
else:
self.secret = secret
if secret is not None and os.path.exists(secret):
with open(secret, 'r', encoding='utf-8') as f:
self.secret = f.read().replace('\n', '')
else:
self.secret = secret

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@@ -97,7 +162,7 @@ def execute_cb(self, goal):
command_type = 'command'
try:
if not self.bots:
self.bots = SwitchBotAPIClient(token=self.token)
self.bots = SwitchBotAPIClient(token=self.token,secret=self.secret)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
self.bots = SwitchBotAPIClient(token=self.token,secret=self.secret)
self.bots = SwitchBotAPIClient(token=self.token, secret=self.secret)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

( I wrote this comment at another unrelated conversation. Sorry. )
A Space added as you suggested.
429cd88#diff-1690cbebb7bf92ad5c7be4524a54f4c0566916a4da9a6b0c493666e891db80c9R165


class SwitchBotAPIClient(object):
"""
For Using SwitchBot via official API.
Please see https://github.com/OpenWonderLabs/SwitchBotAPI for details.
"""
def __init__(self, token):
self._host_domain = "https://api.switch-bot.com/v1.0/"
def __init__(self, token, secret=""):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
def __init__(self, token, secret=""):
def __init__(self, token, secret=None):

"""
Make Request Header
"""
sign, t, nonce = self.make_sign(token, secret)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please keep backward compatibility for v1.0.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Even if a request is sent to SwitchBot API v1.0 with the secret key included in the request header, it behaves like it is just not used and the request seems executed without any problems.

Even so, are there any compatibility issues with API V1.0?
I'm not familiar with Web API, so any advice would be appreciated.


As for another problem about Python 2 compatibility, I changed the code to work in both Python 2 and 3.

429cd88#diff-396497c868baf8630a5e57ccf98059c7d6f1d45e91faccfe0917e1012f124191R45-R66

@k-okada
Copy link
Member

k-okada commented Nov 9, 2023

@sktometometo Are you using jsk_3rdparty/master for your swtichbot experiments? If so, could you please confirm that it does not do any damage to your work?

@sktometometo
Copy link
Contributor

sktometometo commented Nov 9, 2023

I am not using jsk_3rdparty for my current experiement since I am using embed device for switchbot control https://github.com/sktometometo/esp_now_ros/tree/master/sketchbooks%2Fm5atoms3_switchbot_client. But I have used it for elevator experiment. So I will check it.

Copy link
Contributor

@sktometometo sktometometo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This PR works with my experiment.

@k-okada k-okada merged commit 56747c3 into jsk-ros-pkg:master Nov 16, 2023
12 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants