pgoapi icon indicating copy to clipboard operation
pgoapi copied to clipboard

Why Dict?

Open Nostrademous opened this issue 8 years ago • 11 comments

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...

Nostrademous avatar Jul 19 '16 20:07 Nostrademous

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?

joshk6656 avatar Jul 19 '16 20:07 joshk6656

@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

Nostrademous avatar Jul 19 '16 20:07 Nostrademous

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?

joshk6656 avatar Jul 19 '16 20:07 joshk6656

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.

tejado avatar Jul 19 '16 20:07 tejado

@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 avatar Jul 19 '16 20:07 Nostrademous

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
+

Nostrademous avatar Jul 19 '16 20:07 Nostrademous

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...

tejado avatar Jul 19 '16 21:07 tejado

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

Nostrademous avatar Jul 19 '16 21:07 Nostrademous

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.

tejado avatar Jul 19 '16 21:07 tejado

@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.

Nostrademous avatar Jul 19 '16 23:07 Nostrademous

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.

bstpierr avatar Jul 27 '16 14:07 bstpierr