project-owner / Peppy

Peppy Player Repository
GNU General Public License v3.0
72 stars 14 forks source link

Introduction and high level questions. #43

Open dadr opened 1 year ago

dadr commented 1 year ago

I've started exploring Peppy. I see that you have put a lot of work into it and I want to start with expressing my appreciation that you are providing it as FOSS.

Also for the extensive documentation - many thanks!

I've been playing around with the software while listening to internet radio stations. I really like that feature, and that you curated some stations so that the module was useful right away.

So, I am wondering how you would like to get feedback, enhancement suggestions, and even documentation suggestions or corrections. Would you prefer Github Issues, like this one, and is it OK to discuss more general items in the issues section - or would you like to have discussions elsewhere and save issues for changes to the code or docs?

Also, is it preferred to have many smaller and simple issues, or would you rather see several topics in a single issue?

Thanks!

project-owner commented 1 year ago

Hi Tom,

Thank you for the fedback! I really appreciate it. As for questions, enhancements it's up to you. You can open new issue(s). I can label them as a 'question' or 'enhancement' like this one. There are also several forums where you can ask questions or suggest enhancements. Though that needs registration over there: https://www.diyaudio.com/community/threads/peppy-player.288412/page-23 https://forums.raspberrypi.com/viewtopic.php?t=139983&start=425 https://forum-raspberrypi.de/forum/thread/38115-peppy-webradio/?pageNo=68 Some of those forums are more active than others. Again, it's up to you. Probably this place is OK for code proposals. For documentation improvements you can open issue in the Peppy.doc.

Best regards

dadr commented 1 year ago

I have an account on diyaudio, so I can discuss there if you think its a better place, otherwise I will use github.

I'm trying Peppy Klimt on a RPi4B with a HiFiBerry Dac+ and a 5" 800x480 DSI touchscreen - no GPIO buttons or rotary encoders - yet!) I plan to try those later. First it seems that you have a new release, so I will start with the Updating guide.

Do you have a doc or picture that describes the software architecture for Peppy, so that folks who might want to contribute can get an overall picture of what is going on?

Similarly, do you have a reference tool chain you use to develop Peppy?

project-owner commented 1 year ago

I would recommend to use a disk image with a new release instead of updating manually. There is no detailed architecture doc for the player. The following info should give some top level information about the player: https://github.com/project-owner/Peppy.doc/wiki/Software https://github.com/project-owner/Peppy.doc/wiki/Peppy-Player-UI

dadr commented 1 year ago

Well, I did the update. I noticed that there is now a config directory. I also noticed that the player needs the new config.txt file. I tried just copying the old one over, but the player would not start. When I re-edited the new config.txt that worked just fine. It seems that missing config elements are not acceptable.

I will go ahead and try the new release image as well. It's a lot more work to re-edit in everything I've already done, but I've run into a snag where the browser system tab won't show any content anymore. This is not a new thing... I have seen the web config system tab work before, but it stopped showing up in the Klimt release at some point, and I don't see it at all anymore. Updating the software did not bring it back either.

The js console on my browser thinks there is a type error "Fetch Disks problem: l is undefined" That's from Fetchers.js, under Tabs.

project-owner commented 1 year ago

Manual update is a problematic process as very often it needs pretty complicated steps which are difficult to implement in any sort of script. That's why I usually recommend just to use disk images. As you noticed the main problem in this case - to restore your previous changes.

It would be helpful to find a sequence of steps which leads to that 'Fetch Disks problem'. Do you have anything in the file /home/pi/Peppy/nas/nas.txt ? Do you see any error messages in the log file (/home/pi/Peppy/peppy.log) if you enable logging? https://github.com/project-owner/Peppy/blob/c4b55544bbaca5930f74c2d609bc285bc47d7629/configuration/config.txt#L33

Thanks!

dadr commented 1 year ago

My nas/nas.txt file exits with 0 bytes. I turned on the logging, and then captured what got added to the log when I clicked on the System Tab:

[2023-09-28 22:18:22,315] {web.py:2243} INFO - 200 GET /timezone (192.168.1.230) 225.07ms
[2023-09-28 22:18:22,327] {connectionpool.py:1052} DEBUG - Starting new HTTPS connection (1): alphacephei.com:443
[2023-09-28 22:18:22,896] {connectionpool.py:556} DEBUG - https://alphacephei.com:443 "GET /vosk/models/model-list.json HTTP/1.1" 200 None
[2023-09-28 22:18:22,902] {web.py:2243} INFO - 304 GET /voiceassistant/voskmodels?language=English-USA (192.168.1.230) 584.17ms
[2023-09-28 22:18:22,929] {web.py:2243} INFO - 304 GET /alsadevices (192.168.1.230) 26.30ms
[2023-09-28 22:18:22,989] {diskmanager.py:46} DEBUG - []
[2023-09-28 22:18:22,992] {web.py:2243} INFO - 304 GET /diskmanager/disks (192.168.1.230) 59.94ms
project-owner commented 1 year ago

I don't see anything unusual in the player's log. It's difficult to fix an issue which I cannot reproduce. Does the issue occur right away or after some steps in the player or Web UI? Thanks!

dadr commented 1 year ago

I opened a separate issue for the System tab after burning a new image and still having the problem.

I have made a few fixes for some of the radio stations, and added some as well. Would you like an Issue opened with the updated files for that?

Let me also share some good news, I discovered something that I really liked: I tried adding a genre to the Radio module (e.g. Sports). I was pleasantly surprised that this worked - even though I did not see it documented anywhere. At first I had missed creating a folder.svg icon in the Sports directory. The UI painted a Sports selection without an icon, but when I pressed it the UI froze. It's a happy ending, however, because after providing the folder.svg icon, it works great. The UI did a good job of extending the genre selections to a second screen.

project-owner commented 1 year ago

It's OK to have that new issue. There is a short description for radio groups here: https://github.com/project-owner/Peppy.doc/wiki/Radio-Groups#user-defined-radio-groups Though that could be more detailed and include the info about 'folder' images etc.

dadr commented 1 year ago

Not sure if you consider this an issue. If you manually edit the smb.conf to set valid users = %S in a share then peppy wont load. (except for the [homes] share, where this is default). If you try to make this a Option in the web config UI, then it will not save that option. It's really not a big deal.

project-owner commented 1 year ago

Thank you for reporting about these issues. Could you provide the example of the smb.conf file when you change it manually? I'll make sure that it's parsed properly. Also please provide the example of the options which you define in the Web UI. Somehow the parsing logic doesn't handle it properly and should be fixed as well. https://github.com/project-owner/Peppy/blob/c4b55544bbaca5930f74c2d609bc285bc47d7629/util/sambautil.py#L147

dadr commented 1 year ago

I was creating a samba share so I could restore some of the data I backed up from the Klimt version.

I kinda blundered into this problem by setting valid users = %S in a new peppy service. Setting it was a mistake, but from my error I discovered that Peppy fails to load when you use samba variable substitutions in a service definition after the defaults.

Also, if I try to use a variable in the web config - Share Manager - Share Folder - options or path, it is not accepted. (I get a red popup that says "Cannot Save Share")

The service definition below demonstrates the issue. If you type it in the web config it will not save it. If you manually add it to smb.config and then restart samba, you will get a working service for the Peppy folder. And when you run smbutil view //pi@raspberrypi from a client you get an output line that looks like this:

Share                                           Type    Comments
-------------------------------
peppy                                           Disk    peppy on raspberrypi using SMB3_02
[peppy]
comment = %S on %h using %R
browseable = yes
read only = no
create mask = 0770
directory mask = 0770
valid users = pi
path = /home/pi/Peppy

If you then sudo reboot -f that same system will reboot, show the splash screen and then never show the UI. Oddly, I found that if I manually remove the variables from the smb.conf file while the system is in this state, it will recover without having to reboot.

I don't need to use variable substitution on peppy. I blundered into the parsing error and then brought it up because it stopped the system from working.

project-owner commented 1 year ago

So, to reproduce the issue in the Web UI I need to provide the following line in the Options section:

comment = %S on %h using %R

after clicking on the 'Save' button there should be the error in the Web UI.

To reproduce a failed startup I need to add the same line to the smb.conf. Is it correct?

dadr commented 1 year ago

Yes, that's correct.

project-owner commented 1 year ago

It looks like the class ConfigParser which I use to read and write smb.conf file doesn't handle % character by default: https://stackoverflow.com/questions/46156125/how-to-use-signs-in-configparser-python To fix the issue the constructor config_file = ConfigParser(interpolation=None) should be used on the lines: https://github.com/project-owner/Peppy/blob/c4b55544bbaca5930f74c2d609bc285bc47d7629/util/sambautil.py#L89 https://github.com/project-owner/Peppy/blob/c4b55544bbaca5930f74c2d609bc285bc47d7629/util/sambautil.py#L129

The fix will be available in the next release. Thank you!