CyCoreSystems / ari

Golang Asterisk REST Interface (ARI) library
Apache License 2.0
180 stars 74 forks source link

update playback api #143

Closed serfreeman1337 closed 10 months ago

serfreeman1337 commented 1 year ago

See #140 This PR adds playback options and reverts the changes introduced with #142.

 // v5 api (will be still valid with v6 and asterisk v13)
h.Play("myPlaybackID", "sound:hello-world")

// v6 api
h.Play("myPlaybackID", ari.PlaybackOptions{
    Media: []string{"hello-world"}, 
    Lang: "en", 
    OffsetMs: 750,
})
h.Play("myPlaybackID", ari.PlaybackOptions{
    Media: []string{"sound:your", "sound:extension", "sound:number", "sound:is", "digits:123"}, 
})

This change is Reviewable

Ulexus commented 1 year ago

I can appreciate the extra work to maintain compatibility, but my first reaction is that I'd prefer to break compatibility and have a cleaner API (aka no empty interfaces). I'm thinking more functional arguments like we use in ext/play.

Thoughts?

serfreeman1337 commented 1 year ago

Something like that ?

h.Play("myPlaybackID", playback.Media("sound:hello-world"))
h.Play("myPlaybackID", 
    playback.Media("sound:hello-world", "sound:silence/1"),
    playback.Lang("en"), 
    playback.OffsetMs(750),
)

Not sure where to put playback functions.

We can also create alias:

// Array of media URIs to play.
type PlaybackMedia = []string
h.Play("myPlaybackID", ari.PlaybackMedia{"sound:hello-world"}, nil)
h.Play("myPlaybackID", 
    ari.PlaybackMedia{"sound:hello-world", "sound:silence/1"},
    ari.PlaybackOptions{Lang:"en", OffsetMs: 750},
)

Or just leave ari.PlaybackOptions:

h.Play("myPlaybackID", ari.PlaybackOptions{Media: []string{"sound:hello-world"}})
h.Play("myPlaybackID", ari.PlaybackOptions{
    Media: []string{"sound:hello-world", "sound:silence/1"},
    Lang:"en",
    OffsetMs: 750,
})

Other option would be PlaybackOptions as 2nd param and media URI array as variadic:

h.Play("myPlaybackID", nil, "sound:hello-world")
h.Play("myPlaybackID",
    ari.PlaybackOptions{Lang: "en", OffsetMs: 750},
    "sound:hello-world",
    "sound:silence/1",
)