Skip to content

Conversation

@pacjo
Copy link

@pacjo pacjo commented Apr 22, 2025

Hi,
This PR adds support for Smartspacer so that parcels can be displayed in the At a Glance. I've mostly made this for myself, but seeing that it's not that much code, maybe someone else might benefit from it too.

There's more work on my part for this to be ready, but I wanted to ask a couple of questions:

  • is this something you're interested in?

  • is there any one place that triggers updates of the db? for now I'm using notification worker (which... is not good), but maybe I've missed something

  • how's status from the API different from the one stored in DB? (and which one should be preferred?)

Screenshots:

Parcel AfterShip (which I used as a reference)
Screenshot_20250422-153428_Lawnchair Screenshot_20250422-153430_Lawnchair

Also thanks for this awesome project! Looking forward to all the future development.

@pacjo pacjo marked this pull request as draft April 22, 2025 13:50
@itsvic-dev
Copy link
Owner

  1. i'm not sure. i don't use smartspacer myself, but if you (or others) are willing to maintain it in case of breakage, i'm fine with it. just not sure about the extra SDK though tbh
  2. DB updates could be triggered not just from NotificationWorker but also from MainActivity too when opening the ParcelView. this should most definitely be separated out into a separate function to keep business logic separate from UI and NotificationWorker and allow for changes such as this to be slotted in much more easily.
  3. the status stored in the DB is stored only for the purposes of NotificationWorker. they do not differ, in fact it is the same enum. however, you probably should use the one from the APIs if you're fetching parcel data from the API

additionally, minor nitpicks i noticed so far:

  • status should be using the translation key via context.getString(status.nameResource)
  • the status-to-icon logic should prob be moved into a separate file under dev.itsvic.parceltracker.ui

@pacjo
Copy link
Author

pacjo commented Apr 22, 2025

i'm fine with it. just not sure about the extra SDK though tbh

awesome! well... I'm not forcing anything. Should this be merged just ping me if something happens.

DB updates could be triggered not just from NotificationWorker but also from MainActivity too when opening the ParcelView. this should most definitely be separated out into a separate function to keep business logic separate from UI and NotificationWorker and allow for changes such as this to be slotted in much more easily.

Sure. This would mean one background worker that's the source of updates for notifications and smartspacer or two workers?

the status stored in the DB is stored only for the purposes of NotificationWorker. they do not differ, in fact it is the same enum. however, you probably should use the one from the APIs if you're fetching parcel data from the API

alright, I'm fetching them because it's what the notification code did... In the status I wanted to show the text state that API returns. If I can get it from the db that's fine too.

additionally, minor nitpicks i noticed so far:

  • status should be using the translation key via context.getString(status.nameResource)
  • the status-to-icon logic should prob be moved into a separate file under dev.itsvic.parceltracker.ui

yea, I'll remake it properly, but for a quick mod it went quite well.

@itsvic-dev
Copy link
Owner

Sure. This would mean one background worker that's the source of updates for notifications and smartspacer or two workers?

what I meant is that the notification worker and the UI would call one and the same function which fetches the parcel from the API and updates the record for it in the database. right now, that's implemented separately for the notification worker and the UI:

https://github.com/itsvic-dev/parcel/blob/master/app/src/main/java/dev/itsvic/parceltracker/NotificationWorker.kt#L73-L75
https://github.com/itsvic-dev/parcel/blob/master/app/src/main/java/dev/itsvic/parceltracker/MainActivity.kt#L241-L245

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