tejado / pgoapi

Pokemon Go API lib
Other
1.4k stars 445 forks source link

Why Dict? #42

Open Nostrademous opened 8 years ago

Nostrademous commented 8 years ago

Am I the only one who finds writing code dealing with a dictionary way more non-intuitive than if you were returning the ResponseEnvelop directly?

Driving me nuts...

joshk6656 commented 8 years ago

I literally was just typing to ask for an example of how to use the response_dict returned by the api.call().

Nostrademous can you provide any feedback?

Nostrademous commented 8 years ago

@joshk6656 You could do:

    map_obj = response_dict['responses']['GET_MAP_OBJECTS']

    for cell in map_obj['map_cells']:
        for fort in cell['forts']:
            if fort['type'] == 1:
                fort_lat = fort['latitude']
                fort_lng = fort['longitude']
                print('Found PokeStop: (%f, %f)' % (fort_lat, fort_lng))

IF you enabled the "api.get_map_objects()" call

But that is really inconvenient for coding purposes... i would much rather have the protobuf format

joshk6656 commented 8 years ago

So, would I need to do get_map_objects, api.call(), then parse the response, then do another call after? or are they executed in order?

tejado commented 8 years ago

I can't return the complete message with one protobuf object due to the format of the 100 as an example. So there are byte strings in the general response which have to be decoded separately.

But I thought maybe to generate own objects, depending on the response which getter methods etc.

Nostrademous commented 8 years ago

@tejado could you not embed the RpcSub response protos inside the RpcEnvelope Response section and make one proto file?

Then you could do stuff like:

    timestamp = "\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000"        
    cellid = get_cellid(position[0], position[1])                                                             
    api.get_map_objects(latitude=f2i(position[0]), longitude=f2i(position[1]), since_timestamp_ms=timestamp, cell_id=cellid)

    resp = api.call(False) # <-- I added an over-write to dump non-dict responses directly
    if resp.status_code == 200:                                                                               
        p_ret = renv.Response() # "import pgoapi.protos.RpcEnvelope_pb2 as renv" at top
        p_ret.ParseFromString(resp.content)                                                                   
        print(p_ret)

        payload = p_ret.responses[0]
        map_obj = renv.Response..GetMapObjectsResponse()                                                                
        map_obj.ParseFromString(payload)

        for cell in map_obj.map_cells:
           blah blah...
Nostrademous commented 8 years ago

Actually, you don't need to combine them, it works no problem if I do what I suggested, just had one typo in my code.

    timestamp = "\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000" 
    cellid = get_cellid(position[0], position[1]) 
    api.get_map_objects(latitude=f2i(position[0]), longitude=f2i(position[1]), since_timestamp_ms=timestamp, cell_id=cellid) 

    resp = api.call(False) 
    if resp.status_code == 200: 
        p_ret = renv.Response() 
        p_ret.ParseFromString(resp.content) 

        payload = p_ret.responses[0] 
        map_obj = rsub.GetMapObjectsResponse() 
        map_obj.ParseFromString(payload) 

        for cell in map_obj.map_cells: 
            for fort in cell.forts: 
                if fort.type == 1: 
                    print('Found PokeStop: (%f, %f)' % (fort.latitude, fort.longitude)) 

My diff:

diff --git a/pgoapi/pgoapi.py b/pgoapi/pgoapi.py
index 0d2b384..9bee715 100755
--- a/pgoapi/pgoapi.py
+++ b/pgoapi/pgoapi.py
@@ -55,7 +55,7 @@ class PGoApi:

         self._req_method_list = []

-    def call(self):
+    def call(self, dumpToDict=True):
         if not self._req_method_list:
             return False

@@ -75,7 +75,7 @@ class PGoApi:
         self.log.info('Execution of RPC')
         response = None
         try:
-            response = request.request(api_endpoint, self._req_method_list, player_position)
+            response = request.request(api_endpoint, self._req_method_list, player_position, dumpToDict)
         except ServerBusyOrOfflineException as e:
             self.log.info('Server seems to be busy or offline - try again!')

@@ -174,4 +174,4 @@ class PGoApi:
         self.log.info('Login process completed') 

         return True
-        
\ No newline at end of file
+        
diff --git a/pgoapi/rpc_api.py b/pgoapi/rpc_api.py
index 31a39c4..98e7ec3 100755
--- a/pgoapi/rpc_api.py
+++ b/pgoapi/rpc_api.py
@@ -73,7 +73,7 @@ class RpcApi:

         return http_response

-    def request(self, endpoint, subrequests, player_position):
+    def request(self, endpoint, subrequests, player_position, dumpToDict=True):

         if not self._auth_provider or self._auth_provider.is_login() is False:
             raise NotLoggedInException()
@@ -81,9 +81,11 @@ class RpcApi:
         request_proto = self._build_main_request(subrequests, player_position)
         response = self._make_rpc(endpoint, request_proto)

-        response_dict = self._parse_main_request(response, subrequests)
-        
-        return response_dict
+        if dumpToDict:
+            response_dict = self._parse_main_request(response, subrequests)
+            return response_dict
+        else:
+            return response

     def _build_main_request(self, subrequests, player_position = None):
         self.log.debug('Generating main RPC request...')
diff --git a/pokecli.py b/pokecli.py
index aae40c0..8caa124 100755
--- a/pokecli.py
+++ b/pokecli.py
@@ -35,6 +35,9 @@ import argparse
 from pgoapi import PGoApi
 from pgoapi.utilities import f2i, h2f

+import pgoapi.protos.RpcEnvelope_pb2 as renv
+import pgoapi.protos.RpcSub_pb2 as rsub
+
tejado commented 8 years ago

Hm, I'm still thinking about it. What advantages do you see in the protobuf format? Using a dict is not really inconvient if you think about JSON REST API's etc...

Nostrademous commented 8 years ago

@tejado the diff above supports both ways (depending if you pass "False" to the api.call())

tejado commented 8 years ago

No answer to my question? :/

So with your code, you have to parse all subresponses (e.g. GET_MAP_OBJECTS) inside the main response yourself as it is still in a byte format. But this should be the job of the pgoapi lib.

Nostrademous commented 8 years ago

@tejado I don't know if I have a "good" answer to your question. I have an OOP mind-set so writing for loops to iterator based on string-keyed containers is just strange. There are tricks I can do for efficiency too that rely on the "payload" of the packet being in-order, porting to a dict destroys the order and packed format. I realize that you can use an OrderedDict (from collections module) to preserve the order, but honestly, your method is fine for what you are doing. I can do my thing in my own fork...

in any event - you deserve a lot of thanks for all the hard work you put into this, so thank you.

bstpierr commented 8 years ago

I think the less we massage the responses, the closer we are to having a standard format for data.

I think having the API perform the request, parse the responses, and return an array of protobuf response objects is ideal. This allows clients to use the data that's returned from the server directly and removes the possibility of bugs in the protobuf to dict transformation.