-
Notifications
You must be signed in to change notification settings - Fork 67
notifications: iOS support for custom icon/vibe/color #99
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
Conversation
|
cc @sjp4 |
| return entity.toBytes() | ||
| } | ||
|
|
||
| private fun buildVibePatternAttribute(m: StructMapper): Attribute? { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I noticed that we weren't using the same attribute builder here that we are elsewhere in the app (hence lots of code here to reproduce what is already done elsewhere). I've updated the code to use that - if you rebase, hopefully you will be able to just re-use existing build methods (e.g. just calling vibrationPattern {} like here). Ideally everything should go through those APIs when creating attributes (sorry this didn't previously)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks done.
| // return null | ||
| // } | ||
|
|
||
| val iconCode = item.attributes.get(TimelineAttribute.Icon)?.let { bytes -> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The watch can't modify any of these new fields, right? In which case I wouldn't write the code to deserialize them - just always set them all to null here and always use the existing values in handleWrite() in the DAO
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes we cannot edit them on the watch, I was thinking that doing something in the handleWrite() was not the best option so I was expecting that at some point we should deserialize, but if we can do that, yes no point of doing this.
changed to return null
| private fun buildVibePatternAttribute(m: StructMapper): Attribute? { | ||
| return vibePatternName?.let { name -> | ||
| // Try to find default pattern first. | ||
| // TODO: Support custom patterns (requires DB access or passing full pattern here) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should use the VibePatternDao to read all patterns (i.e. not use DefaultVibePattern directly at all). I realize that's tricky right now - I just made a change to make this easier; you'll be able to add the dao to ValueParams, and populate that after injecting it in BlobDb
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thanks I think I did it.
| showBadge: Boolean, | ||
| ) { | ||
| val reallyClickable = clickable && platform == Platform.Android | ||
| val reallyClickable = clickable |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
probably can get rid of the reallyClickable variable
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok removed
| endianness = Endian.Little | ||
| ).toBytes() | ||
| ), | ||
| ) + listOfNotNull( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Did you test this code with an old firmware (e.g. on a pebble time watch or something not-recent)? Asking because the initial android change wound up crashing the watch adding the vibe pattern attribute. We had to add a capability there which gated the phone adding the attribute. If it works, this is fine (it's a different code path on the watch etc so maybe it doesn't hit the same crash), but definitely needs testing.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No I did't test it, but after this changes I tested on my pebble time with old FW and iphone xr and it did't crash.
the watch gets default icon/color/vibration not matter what the phone app sets its ignore them.
28a8b70 to
70ae434
Compare
b0f11df to
8fa98ea
Compare
iOS support for custom icon/vibe/color notifications.
The custom vibe pattern is not supported for the moment, it requires more work, not sure what is the best approach.
It requires an update to the PebbleOS firmware if not this will not take effect on the watch for iOS.
PR: coredevices/PebbleOS#692
Tested on iOS only.