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

Rework getdevicelog() to handle fetching more when "has_next" is True #236

Merged
merged 2 commits into from
Dec 13, 2022

Conversation

uzlonewolf
Copy link
Collaborator

See discussion in #219

@jasonacox jasonacox merged commit 421cffa into jasonacox:master Dec 13, 2022
@jasonacox
Copy link
Owner

Thanks @uzlonewolf !

QQ: Should we add a warning in the README under Cloud that the free Tuya IOT account has restrictions on the number of API calls you can make, and to proceed with care?

@uzlonewolf
Copy link
Collaborator Author

That probably wouldn't be a bad idea.

jasonacox added a commit that referenced this pull request Dec 13, 2022
@bernd-wechner
Copy link

bernd-wechner commented Dec 17, 2022

I just tried this out. Thanks heaps. I'd have one recommendation for data integrity though. When multiple fetches are returned (i.e. the first one returns has_next=True) then the current_row_key entry becomes invalid or meaningless. I see three options to improve the integrity of the returned results (internal consistency):

  1. Remove that key from the result
  2. Set it to None or n/a or some indicator that there is no valid value
  3. Replace it with a list that contains as many values as the 'fetches' key conveys

I lean towards 3.

It's a small issue, as it won't impact what I'm doing one jot. But it just lept out at me as I looked at the first return with 3 fetches.

@uzlonewolf
Copy link
Collaborator Author

uzlonewolf commented Dec 17, 2022

It is my understanding - granted, I have not verified this - that the current_row_key is the key for the very first row returned. As such it should not change even if multiple requests are made to get every requested row since the very first row in the set remains the same no matter how many records are added to the end. This is shown by the current_row_key matching the previous requests' next_row_key.

What are you seeing?

@bernd-wechner
Copy link

I see value for it, but haven't checked what it means, as it's clearly irrelevant and doesn't apply to the logs anymore as they come from multiple rows. Which is why I'd recommend 1, 2 or 3 above. But could add:

  1. rename 'current_row_key' to 'first_row_key'

Which is another option to improve integrity of the response.

P.S. Am working on catching all my logs in a database now, and producing uptime reports for the switches to get a feel for how long they stay up. Essentially my logs come in quadruplets, an online, and offline, a battery_state and a datareport (doorcontact_state) - in part because longer term I'd sort of prefer just to keep the data local and if there's any way in the future I can source by keeping a packet sniffer running on my gateway I'm in - might even block on-tranmission to the Tuya cloud come that day.

@uzlonewolf
Copy link
Collaborator Author

uzlonewolf commented Dec 17, 2022

I'm still not seeing the issue. If you call getdevicelog(..., size=1) getdevicelog(..., size=5) and getdevicelog(..., size=50) they will all return the exact same current_row_key, the only difference is how many log entries are in the result. The loop this PR adds does nothing except extend size= virtually to work around the hard limit of 100 entries. I'm not seeing why the behavior of getdevicelog(..., size=101) should be different than getdevicelog(..., size=100).

As for the key itself, I've got the format mostly worked out. When you base64 decode it, you get 4 fields separated by an underscore. The first field consists of 8 hex chars (4 bytes) followed by the reversed device ID (i.e. if the dev ID is 12345678, the first field will be 0000000087654321). The 2nd field is the maximum positive value of a signed 64-bit int minus the event_time (i.e. if the event_time is 1671240343, the 2nd field will be 9223372036854775807 - 1671240343 = 9223372035183535464). The 3rd field is the event_type, and the last field is the source DPS ID.

@bernd-wechner
Copy link

Wow, you're amazing diligent and awesome! Great reverse engineering.

I figured row_key was a unique way of identifying a "row" whatever that is at the server side. And so is associated with a given result set. Which means "current_row_key" if we're altering the result set (augmenting logs), then it becomes invalid.

As I said, I'm not much fussed as all I want is the logs ;-). Which is behaving oddly at present almost as if its rate limited. When I fetch logs of 4 doors, only the first returns results. Mind you ,I'm experimenting with start and end of 0 and start of 0 and end of maxint. Both seem superficially to return all logs. Long run I'll stick to daily fetches, for now am experimenting with all logs.

@uzlonewolf
Copy link
Collaborator Author

uzlonewolf commented Dec 17, 2022

And so is associated with a given result set. Which means "current_row_key" if we're altering the result set (augmenting logs), then it becomes invalid.

Except it's not, it's the key for the very first row in the set. In addition to size= I mentioned earlier, you can also change start= or evtype= to get a completely different result set and yet it will still have the same current_row_key. As long as the very first row is the same then the current_row_key will also be the same regardless of whatever else changes.

I agree that first_row_key would be a better name, however TinyTuya generally does not rename stuff and deviate from the official Tuya documentation just because a different name would be more descriptive. (See also: "boardcast" instead of broadcast, "ablilty" instead of ability). If you can convince @jasonacox then I will be more than happy to change the code to search for and rename it.

@bernd-wechner
Copy link

Thanks for clarifying, that makes a lot of sense! You're a champ.

Not sure what the boardcast and ablity is about? So I search repo and now I get it. Amazing how clumsy Tuya is.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants