New Model Mapping

[quodrum-glas]

This is not ready to merge, as it requires more typing and testing must be integrated. But it is worth implementing as is faster and easier to maintain I don't have the time to do more, I stripped down testing and typing and use it from my fork (or this without merging many commits). I would not part from it. I tested it extensively from phone app.

This is stripped down from my fork where I have more changes, in order to keep it just to the new model mapping. The lack of time to invest to make this a true pull request, means I keep my changes obscure in my own space.

If you think is worth it, maybe there are other contributors willing to do the necessary testing code, typing, login_hack decoration, etc If this gets integrated, then I will contribute more to the master

PS There might be some bugs, but that would come from the tidalapi I have a update many commits behind here At least the one on page recursion is still happening

[tehkillerbee]

@quodrum-glas Thanks, your contribution is much appreciated, even if it is not a complete PR.

Of course, this will require quite a bit of work to implement your changes but it looks like it could be worth it in the long run. Perhaps @2e0byo or @BlackLight could be interested in having a look. Otherwise, I'd take a look when the latest HiRes features are added to this release of mopidy-tidal

This is stripped down from my fork where I have more changes, in order to keep it just to the new model mapping.

Feel free to link to this other fork, if you have more relevant additions that you think are relevant to add to mopidy-tidal

PS There might be some bugs, but that would come from the tidalapi I have a update many commits behind

Perhaps it might be a good idea to add a similar PR to python-tidal so your bugfixes can be added there so I can get them merged in.

[quodrum-glas]

when the latest HiRes features are added to this release of mopidy-tidal

That indeed is awaited! I tried myself in the past, having PKCE implemented for a while now, but just didn't want to. Probably the identification was not allowed even thought taken from the latest android app. The WebUI credentials showed more promise but got 401 and did not know what to do with it. Now I know it was the player...

Feel free to link to this other fork, if you have more relevant additions that you think are relevant to add to mopidy-tidal

Other items may have become a duplicated at this point. It will will be clearer what's left after the new model and other current effort from you at the moment. But if you have some time, try my fork+branch installation to see the degree of responsiveness The auth is done with a local http server. Maybe is something to help your 2nd part of Fix HI_RES support #147 ?

Perhaps it might be a good idea to add a similar PR to python-tidal so your bugfixes can be added there so I can get them merged in.

Yes, I will start with this. Hopefully starting again using soon to use the master at python-tidal

[2e0byo]

@quodrum-glas thanks for this, it's really good to see.

Couple of high level comments:

• it's really nice to see model conversion in classmethods not functions. I actually chose to stop rewriting the tests when I got to that part recently in the hope that this would happen first. +1 from me on that style
• login_hack is on me. (It's a complete hack, although I think quite cool. It lives in a separate file so I can hide it from the rest of the codebase.)
• intuitively (this is just from a glance) I feel like there are too many properties. Normally when I see lots of properties deferring to an inner type I wonder if there should be another representation (foo.as_bar().x rather than than a property foo.x which returns foo.inner_bar.x)
• most of these models are dataclasses. If I were writing this anywhere else I'd use pydantic. @tehkillerbee what are your thoughts on a library like pydantic in a project like this? The dev experience is very nice. The downside might be installing it on raspberry pi: on the other hand I just ran pip install pydantic on an old rpi2 (zero, 32 bit) and it installed and ran fine: there's a prebuilt wheel.

Functionality-wise caching is definitely a good idea, although we need to handle stale state (if there was handling for that I missed it, sorry).

[sorry for the early post: hit enter by mistake]

[tehkillerbee]

Maybe is something to help your 2nd part of Fix HI_RES support #147 ?

@quodrum-glas Thanks for the suggestion. I ended up implemented it as you suggested, using a local HTTP server based on your code. It works rather well! I had to rewrite most of the relevant backend code but it works and it can also be used for OAuth login.

[quodrum-glas]

@2e0byo Thank you for taking a look at this. Sorry for late reply, I did not expect the attention. Also was on some time off.

The pull request is only to raise awareness on this model mapping that is quite responsive. It is true, I have not used the master version in a long time to compare any advancements that happened there.

Also this model mapping adds more items. It adds pages as per native app browsing, etc. To actually implement it, requires some effort to merge the filtered core from this request, improve, and then add tests, lint it, etc. I can contribute to this within a limited time%.

The cache is defined centrally in one place and can be adjusted easily to best suited balance. The CacheTools has also TTLCache (as implemented at playlist module). I considered that, for example, cache by uri (so track info from id) there won't be any changes server side.

There were a lot of re-requests for data. Especially for track data. Quite expensive.

I welcome the point about pydantic.

I believe the value here is that I tried to match the need for server data with browsing behaviour and frontend structure (as in only request what is needed at the time and no further content without explicit request) , as well as avoiding re-requesting data from server if was supplied earlier in one form or another. (due to incompatibilities in matching exactly server data structure to frontend Mopidy). And that's effectively what I try to "sell" here :) ... the core idea in models.py module and the implementation in playlists.py and library.py

This has lived with me for a while and naturally can be improved already.

[quodrum-glas]

To test this implementation and see the actual outcome, one can go to this ReadMe and use the command as stated there:

sudo python3 -m pip install -U 'git+https://github.com/quodrum-glas/mopidy-tidal.git@quodrumglas-dev#egg=Mopidy-Tidal'

As you know...

python3 -m pip uninstall mopidy-tidal
python3 -m pip uninstall tidalapi

...must be done beforehand

Or use --force-reinstall but will reinstall all dependencies.

tidalapi must be reinstalled from my branch to address a couple of small bugs, especially at Page class A pull request for that fix I submitted here: https://github.com/tamland/python-tidal/pull/236

Another login is required (web server) but won't overwrite the stored session from master

[2e0byo]

@quodrum-glas minor point on the uri: it shouldn't change, but tidal do actually change the uris of resources (perhaps when the rights are granted via a different deal?). I think (but haven't confirmed) that this already bites us.

This looks great and I look forward to testing it. Speedup are very welcome.