feat: Wrapper over notification API #34

Strosel · 2023-01-12T21:20:40Z

Description

A WIP to support notifications without using sys in the user crate

Status

Currently this is a proof of concept for a notification wrapper, however the API needs some polish which has proven difficult due to the types involved. Currently sending a notification requires a [&'static NotificationMessage; N] which is converted into a sys::NotificationSequence via a somewhat messy process including Box::leak. Trying something as simple as wrapping the array in a struct seems to entirely break this process.

There also seems to be a bug (probably due to some internal concurrency or the like) where running the notification example without a terminating thread::sleep causes the flipper to crash.

dcoles

Hi @Strosel,

I saw you opened this draft PR and added some initial feedback. Please let me know if you'd rather me wait until you've marked it as a non-draft.

Thanks for taking this on!

crates/flipperzero/src/notification/mod.rs

crates/flipperzero/src/notification/messages.rs

crates/flipperzero/src/notification/mod.rs

Strosel · 2023-01-15T20:03:28Z

Thanks for all the feedback!

I've tried to update my implementation using your notes, but there's still two roadblocks.

The first is the default messages and sequences, I cant decide between redefining and re-exporting them which is safer but more time-consuming and harder to maintain or implement some kind of from_sys method which would be very unsafe.

The second is the lifetimes of our sequences. With the current implementation the notification example mostly works but will either stop in the middle of the success sequence or crash altogether. It seems like this is because our rust app exits and releases the memory containing the sequence before the firmware has finished working through it. I don't know if this is solvable without leaking memory or using notification_message_block.

dcoles · 2023-01-16T04:01:43Z

The first is the default messages and sequences, I cant decide between redefining and re-exporting them which is safer but more time-consuming and harder to maintain or implement some kind of from_sys method which would be very unsafe.

For some of the sequences (e.g. sys::sequence_success), reexporting them means that they keep in sync with the main firmware (e.g. if they changed the sound). However there's a bit of a trade-off: redefining them might make for a nicer API or protect us from any churn in the C API. I believe you can now use assert!/panic! in const functions, so that might be one way to ensure people don't create a NotificationSequence that's missing a null terminator.

The second is the lifetimes of our sequences. With the current implementation the notification example mostly works but will either stop in the middle of the success sequence or crash altogether. It seems like this is because our rust app exits and releases the memory containing the sequence before the firmware has finished working through it. I don't know if this is solvable without leaking memory or using notification_message_block.

I ran into some issues with this when writing the first version of the notification example. You have to be really careful not to pass a reference to a notification sequence stored on the stack. For example:

// This is OK
let sequence = &sys::sequence_set_only_red_255;
sys::notification_message(notification_app.as_ptr(), sequence);

// This is NOT
let sequence = sys::sequence_set_only_red_255;
sys::notification_message(notification_app.as_ptr(), &sequence);

I'm not exactly sure how Rust handles slices defined within functions (e.g. let x = &[1, 2, 3]), but it's possible a similar thing might be happening. One option would be to try defining the array as a static rather than a const.

If that doesn't work, there might be a fundamental limitation with using sequences defined in an external applications. Hopefully this is not the case.

Strosel · 2023-01-16T12:29:15Z

It would seem that the issue of non-blocking notifications cannot be solved with static as raw pointers are not Send which is a requirement and NotificationSeqence must hold on to at least the terminating null pointer. Therefore I think documenting this behaviour might be the only solution.

Aside from this, I cannot decide on how to layout the NotificationSeqence.

One option is the one present in the last commit, only constructible from a macro to avoid user-generated raw pointers and guaranteeing the terminating Null pointer.
Option two is something like

#[repr(C)]
pub struct NotificationSequence<const N: usize>([&'static NotificationMessage; N], *const ());

the only drawback to this would be the const generic being somewhat restrictive, and will represent a false size (at least until generic_const_exprs is stable). However it could be made generic for other Null terminated arrays if needed in the future.

This seems to be the last blocker so when this is resolved I will mark this PR as non-draft.

dcoles · 2023-01-17T00:33:38Z

It would seem that the issue of non-blocking notifications cannot be solved with static as raw pointers are not Send which is a requirement and NotificationSeqence must hold on to at least the terminating null pointer.

It's possible to have pointers in static, but you need to declare it static mut (which is not particularly obvious):

static mut NOTIFICATION: [*const sys::NotificationMessage; 10] = unsafe { [
    &sys::message_note_c4 as *const sys::NotificationMessage,
    &sys::message_delay_100 as *const sys::NotificationMessage,
    &sys::message_note_e4 as *const sys::NotificationMessage,
    &sys::message_delay_100 as *const sys::NotificationMessage,
    &sys::message_note_g4 as *const sys::NotificationMessage,
    &sys::message_delay_100 as *const sys::NotificationMessage,
    &sys::message_note_c5 as *const sys::NotificationMessage,
    &sys::message_delay_100 as *const sys::NotificationMessage,
    &sys::message_sound_off as *const sys::NotificationMessage,
    ptr::null(),
] };

fn main(_args: *mut u8) -> i32 {
    unsafe {
        let notification_app = unsafe { sys::furi_record_open(RECORD_NOTIFICATION) } as *mut sys::NotificationApp;
        sys::notification_message(notification_app, &NOTIFICATION as *const *const sys::NotificationMessage as *const sys::NotificationSequence);
        sys::furi_record_close(RECORD_NOTIFICATION);
    }

    0
}

This seems to be a little more reliable, but it's still possible to trigger a bus-fault or null-pointer deference after several attempts.

I also was able to reproduce this in a C-based external application.

const NotificationSequence chime = {
    &message_note_c4,
    &message_delay_100,
    &message_note_e4,
    &message_delay_100,
    &message_note_g4,
    &message_delay_100,
    &message_note_c5,
    &message_delay_100,
    &message_sound_off,
    NULL
};

int hello_world(char* /* arg */) {
    NotificationApp* notification_app = furi_record_open(RECORD_NOTIFICATION);
    notification_message(notification_app, &chime);
    furi_record_close(RECORD_NOTIFICATION);

    return 0;
}

So this is probably a firmware bug/race-condition. I'll open an issue on flipperzero-firmware.

Option two is something like [example] the only drawback to this would be the const generic being somewhat restrictive, and will represent a false size (at least until generic_const_exprs is stable). However it could be made generic for other Null terminated arrays if needed in the future.

I do something like this for padding of manifest fields, but that's a little easier since the size of the resulting array is known ahead of time.

But I think your approach with a macro is fine. It's not uncommon to hide unsafe code behind a macro that ensures that it is only used in a safe manner.

This seems to be the last blocker so when this is resolved I will mark this PR as non-draft.

Sounds good to me. I'm excited to have a safe high-level interface for notifications.

dcoles · 2023-01-17T00:53:34Z

Opened flipperdevices/flipperzero-firmware#2313 for the crash when using a custom NotificationSequence.

Strosel · 2023-01-17T00:57:35Z

It's surprisingly relieving that this is a bug in the firmware and not a rust issue 😅

The use of static mut that you propose does, as you point out, allow the use of *const _ in a static however since any and all usage of static mut values is considered unsafe this will not be the safe wrapper I had hoped and intended and brings us back to the question of NotificationSequence layout

Strosel · 2023-01-23T14:41:28Z

@dcoles This is ready for review

dcoles · 2023-01-26T15:36:38Z

@dcoles This is ready for review

Thanks. I should be able to take a look shortly (been a little sick this week).

str4d

Added some thoughts about the Rust-level API, which we can make more expressive than what the Flipper Zero SDK exposes.

str4d · 2023-02-16T02:49:31Z

crates/flipperzero/src/notification/messages.rs

@@ -0,0 +1,61 @@
+use super::{Light, NotificationMessage};


It looks like this module is making certain methods more convenient to use (e.g. NotificationMessage::display_backlight_enforce_auto), but it is also providing some common defaults (e.g. BLINK_START_100) for which a user may have a good reason to call the base method for. It might be beneficial to separate these usages (because the constants that are equivalent to const fns add duplication to the crate API, unless the const fns get made pub(crate)).

The choice of re-exporting the results of the NotificationMessage constructors with no inputs was mainly made to keep parity with the c API. However, I believe that this also increases the user experience somewhat as it enables the user to use a single export for all default messages if they only need and/or want those but I do see your point. What do you think of making the consts that depend on parameters associated constants and scrapping the ones with no parameter to maybe get the best of both worlds?

If there is a desire to keep or provide parity with the C API, that might be better served with a prelude-style module designed for use flipperzero::notification::c_prelude::*; usage, that collects / renames constants from other parts of the API. This would be a fine place for the constants that are duplicative of const fns to live.

As far as designing a "native" Rust API, grouping based on semantics rather than structure feels like it gives a more intuitive API (like my suggestion further down about module renaming). In an ideal world, the user wouldn't need to care much about whether a provided default corresponds to a single NotificationMessage or multiple; they could just compose the messages as they need. We don't live in an ideal world, but macros let us get close.

What do you think of making the consts that depend on parameters associated constants

Associated constants of which types?

Good point but as discussed with play_sound! splatting sequences into each other is very difficult, especially doing so for arbitrary sequences due to slice length etc. therefore I would've preferred to keep messages and sequences separate but since notification_sequence! is type checked I guess it would not be that much of an issue.

I agree that for the flipperzero crate, there is no need for 1:1 party with the C API (some things might be unsafe or have a more idiomatic way to do in Rust). But the intent is to expose as much of features as (safely) possible.

It might even make sense to eventually have an alternate API that's a little more friendly than this one (e.g. Display::set_backlight). NotificationMessage lists aren't exactly the nicest things to work with.

crates/flipperzero/src/notification/mod.rs

crates/flipperzero/src/notification/notes.rs

str4d · 2023-02-16T03:06:24Z

crates/flipperzero/src/notification/notes.rs

@@ -0,0 +1,120 @@
+use super::NotificationMessage;
+
+pub static CLICK: NotificationMessage = NotificationMessage::sound_on(1.0, 1.0);


This constant on its own doesn't produce a click; it is necessary to combine this with a delay and sound_off. It might be more useful to provide this as a notification sequence.

This constant also exists simply for parity like the ones discussed above

str4d · 2023-02-16T04:33:11Z

crates/flipperzero/src/notification/notes.rs

+
+pub static CLICK: NotificationMessage = NotificationMessage::sound_on(1.0, 1.0);
+
+pub const C0: NotificationMessage = NotificationMessage::sound_on(16.35, 1.0);


These all hard-code a volume of 1.0, and AFAICT there is no way to alter this without calling NotificationMessage::sound_on yourself with the same frequency constant (which is opaque).

Instead, we could define these constants using the following API:

/// A pitch at which Flipper Zero can play a sound. #[derive(Clone, Copy, Debug, PartialEq)] pub struct Pitch { frequency: f32, } impl Pitch { /// Defines a new pitch with the given frequency in Hertz. /// /// # Safety /// /// `frequency` must be positive and greater than zero. pub const fn new(frequency: f32) -> Self { Self { frequency } } /// Generates a notification message to start playing sound at this pitch and the /// given volume. /// /// The sound will continue until either another sound replaces it, or [`SOUND_OFF`] /// is used. /// /// # Safety /// /// `volume` must be between `0.0` and `1.0`. pub const fn play(self, volume: f32) -> NotificationMessage { NotificationMessage::sound_on(self.frequency, volume) } } pub const C0: Pitch = Pitch::new(16.35);

and then use it like:

pub const SUCCESS: NotificationSequence = notification_sequence![ messages::DISPLAY_BACKLIGHT_ON, messages::GREEN_255, messages::VIBRO_ON, notes::C5.play(1.0), messages::DELAY_50, messages::VIBRO_OFF, notes::E5.play(1.0), messages::DELAY_50, notes::G5.play(1.0), messages::DELAY_50, notes::C6.play(1.0), messages::DELAY_50, messages::SOUND_OFF, ];

I also experimented locally with an additional API:

/// A sound that Flipper Zero can play. #[derive(Clone, Copy, Debug, PartialEq)] pub struct Sound { pitch: Pitch, duration: u32, } impl Sound { /// Defines a new sound with the given pitch, and duration (in milliseconds). pub const fn new(pitch: Pitch, duration: u32) -> Self { Self { pitch, duration } } /// Generates a sequence of notification messages that will play this sound once at /// the given volume. /// /// # Panics /// /// Panics if `volume < 0.0` or `volume > 1.0`. pub const fn play_once(self, volume: f32) -> SingleSound { ( self.pitch.play(volume), NotificationMessage::delay(self.duration), NotificationMessage::sound_off(), ) } } // Only defined to make the tuple more ergonomic. pub type SingleSound = (NotificationMessage, NotificationMessage, NotificationMessage); const C5_50: SingleSound = Sound::new(notes::C5, 50).play_once(1.0); const E5_50: SingleSound = Sound::new(notes::E5, 50).play_once(1.0); const G5_50: SingleSound = Sound::new(notes::G5, 50).play_once(1.0); const C6_50: SingleSound = Sound::new(notes::C6, 50).play_once(1.0); pub const SUCCESS: NotificationSequence = notification_sequence![ messages::DISPLAY_BACKLIGHT_ON, messages::GREEN_255, messages::VIBRO_ON, C5_50.0, C5_50.1, messages::VIBRO_OFF, E5_50.0, E5_50.1, G5_50.0, G5_50.1, C6_50.0, C6_50.1, messages::SOUND_OFF, ];

It should be possible to rework the notification_sequence! macro to make this ergonomic:

impl Sound { /// Generates a sequence of notification messages that will play this sound for at /// least its specified duration at the given volume. /// /// The sound will continue after its specified duration until either another sound /// replaces it, or [`SOUND_OFF`] is used. /// /// # Panics /// /// Panics if `volume < 0.0` or `volume > 1.0`. pub const fn play(self, volume: f32) -> (NotificationMessage, NotificationMessage) { ( self.pitch.play(volume), NotificationMessage::delay(self.duration), ) } } /// TODO: Change notification_sequence! to support flattening nested tuples or something. const C5_50: Sound = Sound::new(notes::C5, 50); const E5_50: Sound = Sound::new(notes::E5, 50); const G5_50: Sound = Sound::new(notes::G5, 50); const C6_50: Sound = Sound::new(notes::C6, 50); pub const SUCCESS: NotificationSequence = notification_sequence![ messages::DISPLAY_BACKLIGHT_ON, messages::GREEN_255, messages::VIBRO_ON, play_sound!(C5_50, 1.0), messages::VIBRO_OFF, play_sound!(E5_50, 1.0), play_sound!(G5_50, 1.0), play_sound!(C6_50, 1.0), messages::SOUND_OFF, ];

This is a great idea, I was thinking of something similar myself as I have written a toy music crate previously. IMHO the last example seems the best, but I would prefer if the duration and volume were swapped as the volume feels more intrinsic to the note than its duration. Another option would be to give the play_sound macro optional volume and duration arguments with default values.

After playing with this for a while I seem to be stuck on the play_sound macro as it can't "splat" the tuple of messages in a way that makes them usable for notification_sequence! any ideas?

What might be best for now is to keep the explicit API of "sound, delay, new sound/sound off" but add a convenience function for generating sound_on messages with the desired pitch and volume

I would prefer if the duration and volume were swapped as the volume feels more intrinsic to the note than its duration

I had the reverse feeling, but not strongly; the thing I mainly wanted was the existence of Pitch to allow reusing them. Given that NotificationMessage groups them as ((frequency, volume), duration) it also likely makes the macros easier to group them that way.

After playing with this for a while I seem to be stuck on the play_sound macro as it can't "splat" the tuple of messages in a way that makes them usable for notification_sequence! any ideas?

Yeah, I encountered the same issue in my brief period of playing with the API (and so I left it as experimental). I think it might be possible by making play_sound not be a separate macro, but instead a keyword inside the notification_sequence! macro (and then using recursion to parse the macro's arguments left-to-right, and splat the play_sound tuple when we reach it).

What might be best for now is to keep the explicit API of "sound, delay, new sound/sound off" but add a convenience function for generating sound_on messages with the desired pitch and volume

Sounds good 👍

More disappointing news 😕 due to rust-lang/rust#57241 we can't calculate the frequency from some theoretical Pitch enum so I think even a convenience function is impossible at this time. It could be nice to come back to when the issue is resolved though

Hehe, I was already in that issue as a result of this PR, asking if we can have a little const PartialEq, as a treat.

Do we need to calculate frequencies though? I think the existing constant-based approach is fine, just using Pitch constants instead of NotificationMessage constants. If we also want to provide constants for pitch+volume like the C API does, that could be done. But even with an enum approach for a canonical set of sorted notes, the frequencies for them could just be constant.

Could you please clarify how we are to use Pitch constants instead of NotificationMessages when we can not convert one into the other due to restrictions on floats in const?

Pitch::play(self, volume) is precisely this conversion. I don't understand what float calculations you are seeing as necessary?

I think there may have been a misunderstanding on my part. I thought the examples you gave above were more separate and interpreted Pitch in the later ones as something like

enum Pitch { C, Cs, D, ... }

which would need float calculations to generate a frequency.

While implementing Pitch as only a frequency wrapper like above would be possible (and nice) I think the name could be changed to prevent this confusion for other users. Maybe something like this could be implemented instead

#[derive(Debug, Clone, Copy)] pub struct Sound { frequency: f32, volume: f32, duration: u32, } impl Sound { pub const fn new(frequency: f32) -> Self { Self { frequency, volume: 1.0, duration: 0, } } pub const fn volume(mut self, volume: f32) -> Self { self.volume = volume; self } pub const fn duration(mut self, duration: u32) -> Self { self.duration = duration; self } pub const fn play_once(self) -> NotificationMessage { NotificationMessage::sound_on(self.frequency, self.volume) } pub const fn play(self) -> [NotificationMessage; 3] { [ self.play_once(), NotificationMessage::delay(self.duration), NotificationMessage::sound_off(), ] } }

then a constructor using a Pitch enum like above could be implemented in the future and the more challenging part right now would "only" be splatting

str4d · 2023-02-16T04:42:03Z

crates/flipperzero/src/notification/mod.rs

+///Default notification notes.
+pub mod notes;
+///Default notification sequences.
+pub mod sequences;


Rather than grouping all sequences together, the public API might be simpler if we group by kind of notification:

Suggested change

pub mod sequences;

pub mod backlight;

pub mod led;

pub mod sounds;

pub mod vibro;

This seems like a great idea to simplify the API

Per my above reply, I think moving some of the single-message constants currently defined in messages into these "kind-of" modules would also simplify the API. (Rust will already type-check to prevent users from mixing single-message and sequence types where we don't want them to.)

dcoles · 2023-02-27T06:08:12Z

crates/flipperzero/src/notification/messages.rs

@@ -0,0 +1,61 @@
+use super::{Light, NotificationMessage};


I agree that for the flipperzero crate, there is no need for 1:1 party with the C API (some things might be unsafe or have a more idiomatic way to do in Rust). But the intent is to expose as much of features as (safely) possible.

It might even make sense to eventually have an alternate API that's a little more friendly than this one (e.g. Display::set_backlight). NotificationMessage lists aren't exactly the nicest things to work with.

dcoles · 2023-02-27T06:23:11Z

crates/flipperzero/src/notification/mod.rs

+    /// #Safety
+    /// Due to how rust interacts with the firmware this function is not safe to use at any time
+    /// where the application might exit directly afterwards as the rust runtime will free the
+    /// sequence before the firmware has finished reading it. At any time where this is an issue
+    /// `notify_blocking` should be used instead..


Good news! This was resolved in flipperdevices/flipperzero-firmware#2335.

Great! Do you think this may allow us to use dynamically allocated sequences? I would love to test myself but I'm honestly not sure on how to sync this branch with main, I've only ever done so in my own repos where I'm fine with "ugly" merge commits

Unfortunately we'd still need to ensure that the dynamically allocated sequence wasn't free'd before the sequence had finished playing. The fix in flipperdevices/flipperzero-firmware#2335 fixes the case where the sequence is defined statically. What would happen was the program would be unloaded from memory, but the sequence still had a pointer to somewhere in the now free'd memory region. The fix just makes sure the Flipper runtime waits until all previous sequences have finished playing before unloading the program from memory.

The problem for a dynamically allocated sequences is that all this happens after main has exited:

// main { // create dynamically allocated sequence let s = vec![...]; play(s); // non-blocking // s is dropped, causing sequence to be freed } // oops! The flipper runtime wasn't finished with that sequence! // CRASH! wait_for_all_sequences_to_finish();

#49 will result in apps waiting for un-joined threads to finish before exiting the FAP, so I think a similar wait here would be fine (assuming the SDK provides a way for us to determine when it has finished).

I think the eventual answer here will need to be that play(s) takes dynamic sequences by value, and under the hood stores them in an app-static reference (using e.g. #64) to ensure their lifetime extends beyond the user's main(). Then we use the same waiting technique that the firmware does, but inside entry!() instead (like #49 does for threads).

crates/flipperzero/src/notification/mod.rs

dcoles · 2023-03-30T23:24:12Z

Hi @Strosel,

Have you had any chance to address the comments on the PR? Would love to be able to merge it soon.

Strosel · 2023-03-31T10:21:33Z

Sadly I haven't hade time to look at this since March has been a lot busier than expected but hopefully I can look at it in the coming week.

Strosel · 2023-04-12T11:27:39Z

I think, this is ready now. The only thin missing I can think of is a mechanism to "splat" sequences into each other (eg. sounds) but I feel like this could be a feature added later. When/If such a feature is added we could also try to reorganize all messages into their appropriate sequence modules and remove some duplicates that would no longer be needed.

str4d · 2023-05-09T22:38:11Z

This PR needs a rebase to address conflicts with recent changes in main. @Strosel do you want to do this, or would you like me to do so?

Strosel · 2023-05-10T08:58:55Z

Everything should be rebased properly now

str4d

Reviewed as of 80e0ede. I checked that all the constants matched the ones defined in the Flipper Zero SDK.

crates/flipperzero/src/notification/mod.rs

crates/flipperzero/examples/notification.rs

str4d

Lightly tested ACK. Nice work!

dcoles reviewed Jan 13, 2023

View reviewed changes

dcoles added the enhancement New feature or request label Jan 17, 2023

Strosel marked this pull request as ready for review January 18, 2023 17:20

str4d reviewed Feb 16, 2023

View reviewed changes

dcoles self-requested a review February 27, 2023 05:52

dcoles reviewed Feb 27, 2023

View reviewed changes

Strosel force-pushed the notification branch from 47a1dbd to 94bd51a Compare April 5, 2023 22:28

str4d mentioned this pull request May 10, 2023

Release 0.9.0 #65

Merged

Strosel added 8 commits May 10, 2023 10:54

Initial notification implementation

16eb413

repr(transparent) re-impl

428f450

Add default messages, notes, and sequences

c983bf8

Rename notes to sounds and add docs

4ac3024

Reorganize sequences into labled modules

5bece89

Use bitflags for notification::Light

e46e761

Fix names and spelling in notification

91784b2

Fix Light names in notification::messages

80e0ede

Strosel force-pushed the notification branch from b93c948 to 80e0ede Compare May 10, 2023 08:57

str4d requested changes May 10, 2023

View reviewed changes

Fix broken notification example

16c3f4b

str4d approved these changes May 10, 2023

View reviewed changes

str4d added this to the v0.9.0 milestone May 10, 2023

str4d merged commit 684484d into flipperzero-rs:main May 10, 2023

		@@ -0,0 +1,120 @@
		use super::NotificationMessage;

		pub static CLICK: NotificationMessage = NotificationMessage::sound_on(1.0, 1.0);


		pub static CLICK: NotificationMessage = NotificationMessage::sound_on(1.0, 1.0);

		pub const C0: NotificationMessage = NotificationMessage::sound_on(16.35, 1.0);

-pub mod sequences;
+pub mod backlight;
+pub mod led;
+pub mod sounds;
+pub mod vibro;

feat: Wrapper over notification API #34

feat: Wrapper over notification API #34

Conversation

Strosel commented Jan 12, 2023

Description

Status

dcoles left a comment

Choose a reason for hiding this comment

Strosel commented Jan 15, 2023

dcoles commented Jan 16, 2023

Strosel commented Jan 16, 2023

dcoles commented Jan 17, 2023 • edited Loading

dcoles commented Jan 17, 2023

Strosel commented Jan 17, 2023

Strosel commented Jan 23, 2023

dcoles commented Jan 26, 2023

str4d left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

str4d Feb 17, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dcoles commented Mar 30, 2023

Strosel commented Mar 31, 2023

Strosel commented Apr 12, 2023

str4d commented May 9, 2023

Strosel commented May 10, 2023

str4d left a comment

Choose a reason for hiding this comment

str4d left a comment

Choose a reason for hiding this comment

dcoles commented Jan 17, 2023 •

edited

Loading

str4d Feb 17, 2023 •

edited

Loading