GeezerVison

Usually a PR should be focused on one topic or a single feature for example. Some of the changes here should be split off to at least one more PR.

No problem, I have made this PR into the notifications dismissal and now creating the other watch face related one (here: #1151)

I found a bug:
If i send 1 notification and dismiss it, the 'no notifications' "notification" shows and the counter is 0/0 as it should be.
but if I send another notifications and dismiss it, a black screen shows with the counter at 1/2.
sending and dismissing another notification causes the counter total to be incremented.
here is a video:

Nice catch, taking a look at this now. Thanks @Itai-Nelken
EDIT: Odd, it does not happen on InfiniSim, and for some reason my OTA image is not flashing (Pinecone gets to the top after 100%, screen blanks and then it goes back to pinecone. Assumedly the firmware is not happy, and reverts back to verified image).

I have pushed a further change below which fixed an issue with the array assignment when dismissing. I think this should fix your issue with blank notifications.

Happened for me in InfiniSim (create a notification, swipe it away, go back to watchface, swipe down to get to notifications, see total counter is still counted up)

with your fix it doesn't happen on InfiniSim anymore

Mostly fixed. Now if I send 5 notifications and then dismiss them all, same thing happens.

found a dismissal problem. Steps to reproduce

• start simulator
• hit n three times
• dismiss the second (2/3) notification
  
• swipe down to see the oldest notification with wrong index 3/2
  

Doesn't happen if I swipe away the first 1/3 notification. Then the remaining notifications read 1/2 and 2/2

Found a complication, which needs to be handled: The notifications array is used as a ring buffer

• In the beginning readIndex and writeIndex are 0
• after the first push readIndex is still 0, and writeIndex is 1
• writeIndex will always be one more advanced than readIndex
• after the fifth insert readIndex will be 4 and writeIndex has wrapped to be 0
• with the next push we're at readIndex is 0, and writeIndex is 1

found a dismissal problem. Steps to reproduce

* start simulator

* hit `n` three times

* dismiss the second (2/3) notification
  ![InfiniSim_2022-05-19_195359](https://user-images.githubusercontent.com/9076163/169392869-666eafec-4538-4d33-a7f8-0039fd5035b9.png)

* swipe down to see the oldest notification with wrong index `3/2`
  ![InfiniSim_2022-05-19_195408](https://user-images.githubusercontent.com/9076163/169392947-bd035e95-600e-4017-a5a6-52e8e43e654a.png)

Doesn't happen if I swipe away the first 1/3 notification. Then the remaining notifications read 1/2 and 2/2

I think I've fixed this now in my latest commit. Sad news, it's really not the nicest work around. As you've also pointed out, it uses a ring buffer, which means the tail and head are shifting about and indexes and the indexes that are displayed for each notification depends on an ever increasing id. In addition the readIndex and writeIndex now encorperates decrement logic to follow the ring buffer shenanigans.

EDIT:
So my solution was to decrement each valid notification id that is above the dismissed notification id. Afterward, reassign the nextId incrementation based on the highest last id (aka. notifications[readIndex].id + 1).
It now seems to be behaving predictably.

Works well for me.

Found a way to break the current code:

• start simulator
• hit n 7 times (create seven notifications)
• swipe down to get to the notifications
  
• swipe away two times
• 0/0 notifications shown
  

Found a way to break the current code:

* start simulator

* hit `n` 7 times (create seven notifications)

* swipe down to get to the notifications
  ![showing 1/5 notifications before swiping away](https://user-images.githubusercontent.com/9076163/171059433-a7d24080-2f30-471e-9231-b0a3d648b8f6.png)

* swipe away two times

* 0/0 notifications shown
  ![showing 0/0 notifications](https://user-images.githubusercontent.com/9076163/171059309-b4becc97-c6a4-48ce-af23-7457d51520fd.png)

Having a go at this... This is tripping me up quite a bit!
The issue also happens for n == 6.

It seems that:

• n == 7: readIndex = 1
• dismiss 1: readIndex = 0
• dismess 2: readIndex loops back to 4.
• Then notifications[4] is an empty element, and shows it. As notifications[TotalNbNotifications - 1] = {} every dismiss. Which seems to be the problem...

In order to fix this, I need to be able to dismiss based on readIndex's position, not just from dismissed element to end of array... I think.

11111  // n == 7
 ^
11110  // dismiss x1
^
11100  // dismiss x2
    ^

Cooking something up... If anyone has any ideas, let me know!

I've created a SmallRingBuffer class to hide the ringbuffer index handling inside. And outside the index from 0 to 4 can be requested. You're welcome to test it :)

It needs some restructure and cleanups, but I think it is working

edit: cleaned up: NeroBurner@ef0facf

@Tiggilyboo I liked the swipe to dismiss feature so much I opened a new PR #1173 with the code from this PR, rebased it and added the ring-buffer rework. Is it ok for you to move to the new PR, or would you like to update this PR instead and champion the changes yourself? Either one is fine by me, I just want the notification dismissal feature 😁

@Tiggilyboo I liked the swipe to dismiss feature so much I opened a new PR #1173 with the code from this PR, rebased it and added the ring-buffer rework. Is it ok for you to move to the new PR, or would you like to update this PR instead and champion the changes yourself? Either one is fine by me, I just want the notification dismissal feature 😁

Yeah for sure get it merged, afk this weekend otherwise I would. Thanks for the ring buffer fixes!

You're very welcome! :)

Then I'm closing this PR in favor of my new one.