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

Surface Reminder Time Stamp #129

Closed
HiJohnElliott opened this issue Dec 26, 2024 · 15 comments
Closed

Surface Reminder Time Stamp #129

HiJohnElliott opened this issue Dec 26, 2024 · 15 comments

Comments

@HiJohnElliott
Copy link
Contributor

Goal

  • As a relatively junior programmer.
  • I want to be able to surface the reminder time for todos in the Today view in Things.
  • so that I can connect this data with things like calendaring systems that require a timestamp.

Suggested approach
Ideally, surfacing this data would take place in its own field like the other time stamps as something like 'reminder_time' alongside the other timestamps like 'start_date'. Alternatively, the timestamp could be added to the 'start_date' field in instances where one exists. For example, as of now it shows only the date as '2024-26-24' but could show '2024-26-24 11:00:00' for an 11am reminder time.

Additional context
Near as I can tell, this data is stored in the TMTask table under the reminderTime column of the Things DB. I am having a hard time figuring out the encoding and if this is a "Things date" or something else.

Sincerest apologies in advance if I am missing something and this data is already accessible in a way I have not yet found!

@mikez
Copy link
Contributor

mikez commented Feb 11, 2025

@HiJohnElliott Thank you for the reminder timestamp idea. That's this field below, right?

Image

I can confirm that this data is not surfaced at this time. I also think your initial idea of a "reminder_time" key makes most sense (to me) to keep things consistent with the app itself and the database.

Should you choose to do a pull request, I'm happy to review. Contribution pointers here.

I am having a hard time figuring out the encoding and if this is a "Things date" or something else.

Could you post a couple of examples — maybe I can detect it?

@HiJohnElliott
Copy link
Contributor Author

Hey mikez!

Thanks so much for the response! You are totally correct that the Reminder field in your screenshot is the one that I would love to surface with a "reminder_time" key.

I took a little time this weekend to take another crack at how the encoding for the reminderTime field works on the TMTask table of the DB. It turns out that it is sort of a sister encoding of the startDate "Things Date" encoding. In those cases, the bits use the "YYYYYYYYYYYMMMMDDDDD0000000" pattern where the year, month, and day are encoded in binary chunks. Near as I can tell, the reminderTime is encoded as "HHHHHHmmmmm00000000000000000000" where m is minutes and H is hours. This simple function seems to be successfully decoding the reminderTime integer in to hours and minutes:

def simple_things_time_decode(things_time: int) -> str: 
    hours = ((things_time >> 25) & 0b1111111) // 2
    minutes = ((things_time >> 20) & 0b11111)
    return f"{hours}:{minutes}:00"

Inputing 1302331392 should result in "19:26:00" or 7:26pm in 12-hour time. Here are a few more integers from the DB and the ISO times they should convert to:

  • 1436549120 should be "21:26:00" or 9:26pm
  • 1211105280 should be "18:3:00" or 6:03pm (note that there isn't yet any formatting to make sure that returns two-digit values like "01", "02", etc.

I might have some time this week to put together a pull request that includes a convert_thingstime_sql_expression_to_isodate() function and see if I can tie it in to the rest of the library. That being said, if you feel like this would be an easy add then you can totally run with this.

Thanks again mikez! I'm excited to make use of this data!

j Ξ

@mikez
Copy link
Contributor

mikez commented Feb 17, 2025

@HiJohnElliott! Thank you for your analysis. Looks great. Happy to review your pull request.

Minor comments:

  • On my machine, the format appears to be hhhhhmmmmmm00000000000000000000
def simple_things_time_decode(things_time: int) -> str:
    hours = (things_time >> 26) & 0b11111
    minutes = (things_time >> 21) & 0b111111
    return f"{hours}:{minutes:02}"
  • convert_thingstime_sql_expression_to_isotime: note that hh:mm seems to be fine in unambiguous contexts according to spec ... the thingstime appears to not be fixed to any particular timezone (unlike thingsdata which is UTC).

@mikez
Copy link
Contributor

mikez commented Mar 13, 2025

@HiJohnElliott Are you still interested in pursuing this?

@HiJohnElliott
Copy link
Contributor Author

Hey Mikez!

I am indeed still interested in putting together a PR for this. My apologies for the long wait.

I took some time last weekend to trace back the ways in which the thingsDate goes from the bit-packed integer in the DB to the final product in the API and it has turned out to be a little more complicated than I thought. By my accounting, here is the list of functions that the thingsDates passes through that need to be replicated for reminderTime:

  1. convert_isodate_sql_expression_to_things_date()
  2. isodate_to_yyyyyyyyyyymmmmddddd()
  3. match_date()
  4. is_instance()
  5. make_thingsdate_filter()
  6. Database.get_tasks()
  7. tasks()
  8. API.Things.today()

I am adding functions now for:

  1. convert_isotime_sql_expression_to_things_time()
  2. isodate_to_HHHHHmmmmmm()
  3. match_time()
  4. make_thingstime_filter()

If any of these are unnecessary or you can think of a shorter route just let me know wha you think and I'd be happy to comply. In each case, I am trying to math the existing code as closely as possible so that everything

I am definitely making progress and am shooting to have something ready to submit in the next week.

@mikez
Copy link
Contributor

mikez commented Mar 14, 2025

@HiJohnElliott I think you can skip the filters and just focus on modifying COLUMNS_TO_OMIT_IF_NONE, make_tasks_sql_query and add a convert_thingstime_sql_expression_to_isotime function. That should suffice. – That way a "reminder_time" will be returned if available, but you can't filter for it (which I guess isn't needed in practice where you can do that with Python directly if needed). Does that make sense?

@HiJohnElliott
Copy link
Contributor Author

This makes perfect sense! I really appreciate the help here. I am jumping on this now.

@HiJohnElliott
Copy link
Contributor Author

Hey mikez! I have been taking some time with this and have made the suggested changes but I'm not seeing the reminder_time value show up yet when calling any of the Utility API functions. Here's what I've done so far:

  • Added "reminder_time" to COLUMNS_TO_OMIT_IF_NONE
  • Added a `convert_thingstime_sql_expression_to_isotime function
  • Added this function to make_tasks_sql_query and also added reminder_time to the SELECT statement in that same function.

I've been polking around to see where the hold up is but haven't had any luck. I have a branch going locally that I would be happy to share if that would be helpful.

Apologies that I haven't been able to solve this one on my own yet!

@mikez
Copy link
Contributor

mikez commented Mar 15, 2025

@HiJohnElliott If you're comfortable putting a forked branch on your local GitHub account, I'll take a look. 👍

@HiJohnElliott
Copy link
Contributor Author

Hey Mikez!

Happy to. Here is the link to the reminder_time branch of my fork of the repo:

https://github.com/HiJohnElliott/things.py/tree/reminder_time

Thanks again for taking a look at this! I'm sure that I am missing something really easy so your expertise here is greatly appreciated.

@mikez
Copy link
Contributor

mikez commented Mar 16, 2025

@HiJohnElliott Thank you for the link.
Did you try adding the flag print_sql=True when you run one of the API functions? This will show you the SQL statements being run. You can then use a SQL tool of your choice to tweak the SQL command until it returns what you'd expect it to return.

@HiJohnElliott
Copy link
Contributor Author

Hey Mikez,

Thanks for pointing out this print_sql=True parameter! It helped me track down my issue to something in my environment.

I'm excited to say that I've been doing some testing and that it looks like the reminder_time values are now showing up as expected for those tasks that have a reminder time set.

Now that the code seems to be working, I will have to update the tests and test DB with some valid reminderTime data to verify that everything is working as intended. Would you prefer that I submit the current code and the updates to the tests as separate PR's or would you prefer that I submit them all together as one PR?

Thanks Mikez!

@mikez
Copy link
Contributor

mikez commented Mar 18, 2025

@HiJohnElliott Feel free to submit it all in one PR: code updates and test updates. I'll then do a code review.

@mikez
Copy link
Contributor

mikez commented Mar 25, 2025

@HiJohnElliott And if you get stuck or need pointers, don't hesitate to reach out!

@HiJohnElliott
Copy link
Contributor Author

Hey @mikez! I think I put in the PR last night.

It can be found at #133.

That being said, I wouldn't be surprised if I didn't get this PR in the right place so pointers would be very welcome.

Thanks so much!

@mikez mikez closed this as completed Apr 1, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

2 participants