[messages] is 'music' export needed?

[gfwilliams]

@rigrig I've just been looking through the messages library and I see we still have music export with the currently playing track info in.

But as far as I can see, this isn't used by anything now? Any playing music is treated as a new item in the messages list (which messagesgui uses), and since we're only saving it on exit now, it's not a big deal for us updating it.

Or am I missing something?

[rigrig]

It's used to combine GB({"t":"musicinfo" and GB({"t":"musicstate" messages: we get musicinfo for every track change, but musicinfo only when music starts/stops.
This way (almost) all {id:"music"} messages have both "state" and "track" fields present.

[gfwilliams]

Ok, thanks! Are you sure that's needed now though? Because it looks like we should really just store what's needed in the messages list?

... or maybe if it is needed, that code should go into the Android library as I guess it's not needed for iOS

[rigrig]

that code should go into the Android library

You're right, ios already pushes {id:"music"} directly, so I guess the whole musicstate/musicinfo split should just be handled inside android.

And looking at ios, I wonder whether it even includes "state" info? (It uses NRF.amsGetMusicInfo(), which isn't in the documentation...) If not, I suspect auto open might just be broken for iOS, because the UI bootcode waits for both state and title to be present.

[gfwilliams]

Yes, that's interesting - I don't know about iOS - maybe that doesn't work...

With the music var though, I'm still not sure it's needed? Without it, you get the two messages pretty close, so surely you'd get:

GB({"t":"musicstate","state":"play","position":0,"shuffle":1,"repeat":1})
// messages now contains {"id":"music","state":"play","position":0,"shuffle":1,"repeat":1}
GB({"t":"musicinfo","artist":"My Artist","album":"My Album","track":"Track One","dur":241,"c":2,"n":2})
// messages now contains {"id":"music","state":"play","position":0,"shuffle":1,"repeat":1,"artist":"My Artist","album":"My Album","track":"Track One","dur":241,"c":2,"n":2}

Or similar - I can't remember exactly what goes in, but you get the idea. Even if you jump to the messages app on the first message, you'd hope it would handle the changed message and then decide it should then display it?

[rigrig]

With the music var though, I'm still not sure it's needed?

Oh right, it already combines them because they are {t:"modify"}. You're right, we don't need to keep special track of music.
(I think it's a leftover from trying to only emit/handle music with both state and info, but UIs should just be able to handle "incomplete" messages)

I can't remember exactly what goes in

Me neither, that's why I added sample_messages.js a while ago 😉 (should probably move that to android)

[gfwilliams]

Ok, great. I'll leave this issue open then and will have a go at getting rid of that.

Me neither, that's why I added sample_messages.js

I didn't spot that! I have one (not committed!) here as well.

In the Espruino repo we have something where some actual tests are run on the factory apps in an emulator: https://github.com/espruino/Espruino/tree/master/tests/FactoryApps

How that's done isn't ideal, but I can't help thinking that we should probably have something in BangleApps - and specifically the whole messages system is so big it'd be really nice to have a clear set of tests so it was obvious if something broke.

[rigrig]

specifically the whole messages system is so big it'd be really nice to have a clear set of tests so it was obvious if something broke.

Yes, that would be nice.

[gfwilliams]

I've just added the basis of something at b89a368bd996b3cbfdbf728c9af8d9fa51329472 but not sure I'll get much further with it this year - hopefully at some point we'll have something usable though