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

Add initial TOTP watch face impl #23

Merged
merged 9 commits into from
Nov 24, 2021
Merged

Conversation

tahnok
Copy link
Collaborator

@tahnok tahnok commented Nov 23, 2021

Vendor code from https://github.com/Netthaw/TOTP-MCU to do the
heavy lifting of computing SHA-1 and HMAC and the rest of TOTP

Also implement a date_time to unix timestamp method

Vendor code from https://github.com/Netthaw/TOTP-MCU to do the
heavy lifting of computing SHA-1 and HMAC and the rest of TOTP

Also implement a date_time to unix timestamp method
@tahnok
Copy link
Collaborator Author

tahnok commented Nov 23, 2021

A few things that are probably worth discussing here:

  1. Is vendoring the code from https://github.com/Netthaw/TOTP-MCU better than a submodule? I think probably yes for simplicity, unless more submodules get added
  2. Do you want to pull the current_unix_time_from_rtc code into a time library?
  3. I think it would make sense to add a UTC offset to the settings (it's used by the beats face as well)
  4. It might make sense to think about some kind of config / settings file that is compiled into the watch for injecting the secret, but would also be nice for setting locations or other complex data that isn't going to change much? Maybe just worth keeping an eye out for other things that could benefit from some compile-time editable settings

@joeycastillo
Copy link
Owner

  1. I'm sold on using it as a submodule, as long as we can figure out the additions to the Makefile that would allow submodules to be pulled in automatically when building.
  2. Definitely, and in fact I may get that done today! I'll likely change the function name though just to namespace it.
  3. Hard agree; my challenge has been figuring out some of the weird edge cases (like offsets that are 5:30 or 5:45 off from UTC). My current thinking is to have a const global array of the 38 existing time zone offsets, and to dedicate six bits of the movement settings struct to an index within that array. (nobody should need more than 64 time zones)
  4. This does make sense; in fact I just had to deal with a similar thing in compiling in a thermistor configuration. While this is a bit further out, we can configure our linker file to reserve an EEPROM emulation area at the end of flash (sized from 256-16384 bytes). This would allow us to stash configuration data in a spot that survives being unplugged and stuffed into the watch case. In a conversation with @wormyrocks a few weeks back, he suggested using a WebUSB interface to write configuration data when plugged in to USB. Definitely further out, but I like that idea as a way to configure these more complex settings.

@joeycastillo
Copy link
Owner

I tested your UNIX time conversion and it looks good! I would definitely like to put this functionality in the watch library. Rather than me pasting the code in, I figure it's preferable for you to add it, to preserve authorship? I added documentation in watch_utility.h and a stub in watch_utility.c; you can just move your function over there (the parameters are all the same).

I also found if you re-add the timestamp -= utc_offset line that you had alluded to in an earlier revision, it seems to handle the UTC offsetting quite nicely!

@tahnok
Copy link
Collaborator Author

tahnok commented Nov 24, 2021

Cool, I've filled out the stub function, thanks for thinking about preserving authorship 😄

I also properly vendored the TOTP-MCU dependency, I had added it as a submodule and attempted to revert that, but I messed it up. I think it can be added back as a submodule that gets pulled automatically as part of the Makefile, but I'm too tired and new to make to do that well right now. Maybe in a follow up PR?

@tahnok
Copy link
Collaborator Author

tahnok commented Nov 24, 2021

This does make sense; in fact I just had to deal with a similar thing in compiling in a thermistor configuration. While this is a bit further out, we can configure our linker file to reserve an EEPROM emulation area at the end of flash (sized from 256-16384 bytes). This would allow us to stash configuration data in a spot that survives being unplugged and stuffed into the watch case. In a conversation with @wormyrocks a few weeks back, he suggested using a WebUSB interface to write configuration data when plugged in to USB. Definitely further out, but I like that idea as a way to configure these more complex settings.

That sounds cool, I wonder if you could have the initial version just be an interactive setup thingy over USB serial, like how the buspirate is configured?

@tahnok tahnok marked this pull request as ready for review November 24, 2021 02:46
@tahnok
Copy link
Collaborator Author

tahnok commented Nov 24, 2021

Ok, marked this as ready to merge. I can follow up to have support for multiple codes, since (IMO) small PRs are good PRs

@tahnok
Copy link
Collaborator Author

tahnok commented Nov 24, 2021 via email

@joeycastillo
Copy link
Owner

I can make that change! I also need to make a couple of small tweaks to the TOTP library; need to move some of sha1's declarations from the header to the source file, because it's causing linker issues with my version of GCC.

@joeycastillo joeycastillo merged commit 4a71def into joeycastillo:main Nov 24, 2021
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.

2 participants