Feat : change specie detected

[alexbelgium]

Objective : provide a way to change a misidentified detection

Status :

• [x] script creation to change database and move files
• [x] initial working function to call script by passing both the filename, and the new expected specie
• [x] nice overlay for the dropdown list that works on every page

Implementation :

• User interface: new button that calls a change detection specie
• Click opens a dropdown list as an overlay with a user input box to filter
• Then asks confirmation

How :

• The function calls a shell script that changes both the database (scientific name, common name, filename) and moves the files if existing to the correct place
• Use of modal box for overlay

[alexbelgium]

Ok seems to work both on web and mobile

I'm testing it with :

# Clean previous files
rm /home/pi/BirdNET-Pi/scripts/play.php
rm /home/pi/BirdNET-Pi/homepage/style.css

# Download new files
curl -o /home/pi/BirdNET-Pi/homepage/images/bird.svg https://raw.githubusercontent.com/alexbelgium/BirdNET-Pi/patch-1/homepage/images/bird.svg
curl -o /home/pi/BirdNET-Pi/scripts/birdnet_changeidentification.sh https://raw.githubusercontent.com/alexbelgium/BirdNET-Pi/patch-1/scripts/birdnet_changeidentification.sh
curl -o /home/pi/BirdNET-Pi/scripts/play.php https://raw.githubusercontent.com/alexbelgium/BirdNET-Pi/patch-1/scripts/play.php
curl -o /home/pi/BirdNET-Pi/homepage/style.css https://raw.githubusercontent.com/alexbelgium/BirdNET-Pi/patch-1/homepage/style.css

# Correct permissions
chmod 777 /home/pi/BirdNET-Pi/scripts/birdnet_changeidentification.sh
chmod 777 /home/pi/BirdNET-Pi/scripts/play.php
chmod 777 /home/pi/BirdNET-Pi/homepage/style.css

[alexbelgium]

I've corrected the curl paths above, they were not correct

[lloydbayley]

I have this PR up and running on a dev BirdNet install, so hopefully tomorrow I will have some detections to play around with! :)

[alexbelgium]

@lloydbayley : did you install the PR with the code I was showing above using curl ?

If we break down by elements :

Working in your test :

• image appears
• function is called
• modal css is displayed
• PR with similar filtering function is working : https://github.com/Nachtzuster/BirdNET-Pi/pull/69

  Not working in your test :

• [ ] Empty selection box
• [ ] Cancel modal box by Esc or click in blank

I'll push some potential fixes that could make it work on my system and not yours

[alexbelgium]

I think the path for labels was specific for my addon, please let me know if this now works! cheers

[lloydbayley]

Sorry, was overtaken by events. Will try it now!

[lloydbayley]

Well, the first part works but (YIPEE) but when I click to change it, I get:

In a browser popup.

birdnet_change_identification.sh has permissions of -rw-r--r-- and is owned by the user running birdnet pi

[alexbelgium]

thanks ; that's due to php being executed by user caddy and on my addon i gave "caddy" and "pi" the same accesses 1000:1000 to avoid such issues :-) I'll see how to adapt and execute using a sudo -u pi command

[lloydbayley]

Just remember that the user may not necessarily be 'pi'. I like the bend the rules on Dev machines and have a few things non-standard to see what happens when it goes outside the box! :)

[alexbelgium]

Should be good! Now user pi will execute the script, which also allows to make sure that the script has only access to "pi" files

Edit : just read your comment and indeed it used the get(user) to make sure it is not fixed

[lloydbayley]

Getting there. Small typo that yields: Error : sudo: /home/pi/BirdNET-Pi/scripts/birdnet_changeidentification.sh: command not found<br> You've left a _ out of change_identification!

[alexbelgium]

argh I'll correct ; thanks

[alexbelgium]

Actually the path was correct ; perhaps it was because there was no exec command. I'll add it. Or missing permission on the sh file for user pi ?

[lloydbayley]

Sorry:

Error : sudo: exec: command not found<br>

[alexbelgium]

I see - the issue is actually due to $home not being defined I'll correct that (now why was it working on my system...)

[lloydbayley]

Lots of fun, this debugging thing isn't it? Ha

[alexbelgium]

ahah crazy it's always the small variations from system to system ;-) but I really appreciate your time there as it will much robust the code !

[lloydbayley]

Sorry Alex, same:

Error : sudo: exec: command not found<br>

[alexbelgium]

mmh don't you have bash installed? Perhaps I should use sh and that the issue from the start was the shebang of the script

Edit : better, I'll align the shebang with other scripts

[lloydbayley]

I certainly do have bash installed. Just a normal bash shell. Also noticed you have hardcoded /home/pi at the top of the script (not that it makes any difference to the current problem)...Just pointing it out.

[alexbelgium]

thanks, HOME is corrected to ${HOME:-/home/pi} ; that should cover all user usages

[lloydbayley]

Gimme a sec...you're pushing them faster than I can revert and re-pull! :)

[lloydbayley]

Same again, I'm afraid...Although, it did think about it for a fraction of a second longer. Also, just looking (I know the shebang takes care of it but the permissions of all the other .sh are 777 (although, I tried that manually and it still shouted at me!)

[alexbelgium]

mmh... I don't understand why it doesn't work on your system and it does on mine. especially as I'm using the same logic (calling $user and $home) as being used in other scripts... and the worst is that initially it worked !

So the issue came when when we started calling the "sudo -u $user" to access the script it seems...

[lloydbayley]

I'm just debugging the php call to the script on my end. I think that's where it's going wrong...

[lloydbayley]

Whilst I'm checking, have you done anything strange with visudo or sudoers in your environment that wouldn't be in a normal install? I'm just debugging what is being sent to bash from php on my end.

[alexbelgium]

I did add everyone to sudoers echo "$USER ALL=(ALL) NOPASSWD:ALL" >> /etc/sudoers https://github.com/alexbelgium/hassio-addons/blob/03e9ad44329c826b8144fac0943659610b32e1e1/birdnet-pi/Dockerfile#L54

But at the same time other scripts are also called using the same logic such as : shell_exec("sudo -u".$user." git -C ".$home."/BirdNET-Pi fetch > /dev/null 2>/dev/null &"); https://github.com/Nachtzuster/BirdNET-Pi/blob/e0e5aff08175c9d0678ee8d8755da04d7c996a70/homepage/views.php#L16C3-L16C94

[lloydbayley]

HANG ON......Why is the word exec being put to the bash shell? ("sudo -u ".$user." exec ".$home."/BirdNET-Pi/scripts/birdnet_changeidentification.sh \"$oldname\" \"$newname\" log_errors 2>&1"); That shouldn't be there, surely...that's why it's trying to execute exec. Let me take it out and try

[lloydbayley]

BINGO. That was it. ("sudo -u ".$user." exec ".$home."/BirdNET-Pi/scripts/birdnet_changeidentification.sh \"$oldname\" \"$newname\" log_errors 2>&1"); Should read: ("sudo -u ".$user." ".$home."/BirdNET-Pi/scripts/birdnet_changeidentification.sh \"$oldname\" \"$newname\" log_errors 2>&1");

[lloydbayley]

Well bugger. You're too quick off the mark. Will retest with latest commit.

[alexbelgium]

BINGO. That was it. ("sudo -u ".$user." exec ".$home."/BirdNET-Pi/scripts/birdnet_changeidentification.sh \"$oldname\" \"$newname\" log_errors 2>&1"); Should read: ("sudo -u ".$user." ".$home."/BirdNET-Pi/scripts/birdnet_changeidentification.sh \"$oldname\" \"$newname\" log_errors 2>&1");

Perfect!!!!! thanks very much I'll align final code with this line

[lloydbayley]

That last commit was worse. :) Error : sudo: /home/userhere/BirdNET-Pi/scripts/birdnet_changeidentification.sh: command not found<br> I'm not doing any more until you tell me you're ready!

[alexbelgium]

Well actually I've aligned the code with your proposal... so it should work in theory ? at least the code is aligned with your proposal in https://github.com/Nachtzuster/BirdNET-Pi/pull/62#issuecomment-2122702645

[lloydbayley]

Ok...So I test now? :)

[alexbelgium]

yes please :-)

the only difference I see from your working code is that you have : ("sudo -u ".$user." ".$home."/BirdNET-Pi/scripts/birdnet_changeidentification.sh \"$oldname\" \"$newname\" log_errors 2>&1"); while my current code is ("sudo -u ".$user." ".$home."/BirdNET-Pi/scripts/birdnet_changeidentification.sh \"$oldname\" \"$newname\" log_errors 2>&1", $output)) should the ";" be there? I don't see it's function in this code?

[lloydbayley]

It's ok. Was leftover from when I was debugging in a php -a session locally and I didn't test for $output because it wasn't important in that test, so yet, that's correct. Will try it now.

[lloydbayley]

Ok. We're 99% there. It failed because the sh file is still not 777. Did that manually and re-tested and it works perfectly.

[lloydbayley]

God....I've pushed him too hard and killed him!

[alexbelgium]

That's so great thanks very much!!! So the current code in play.php works??

[lloydbayley]

Yep. Just need to fix the file permissions!

[alexbelgium]

I don't see specific chmodding if the scripts in the main code ; I think it is managed through chmod -R g+rw $my_dir in https://github.com/Nachtzuster/BirdNET-Pi/blob/a22c7c089fa94eb56df04d2b5812d28ef379bba4/scripts/install_services.sh#L85

So if the PR is pushed the permission should be set hopefully in the same way as the other scripts

[lloydbayley]

Well, it is created with 644 and when I changed it to 777, it started working. That's all I know!

[alexbelgium]

Thanks I wonder then why the other scripts are at 777... Well anyway i'll see how to plug-in this :)

Thanks very much for your availability and the very dynamic troubleshooting!!

[lloydbayley]

I'm wondering why they are as well, however, I didn't go too far into it :) No worries. Happy to help!

One little suggestion, perhaps, if I may... Upon successful completion, perhaps a little successful message of some sort?

Currently, if that was the only recording, it just switches to No Recordings Found with it still showing the old name and IF there are multiple recordings, it shows the next one.

I think some sort of success message would be good.

[alexbelgium]

Sure ! I'll have to go but will check that tomorrow ;)

[lloydbayley]

Fair enough! I should go to bed here! Have a good evening.

[alexbelgium]

Permissions

Should be taken care of by the "install" or "update scripts"

EDIT : i've removed the intial text that might be misleading ; the next 2 messages in this thread refer to this misleading text

Ok message

Added a "Successfully converted" message

[lloydbayley]

How do you mean people will have to update their installation with newinstaller.sh? When the internal update happens, it will call birdnet_update and the snippets updater. I'm not with you I'm afraid. Have I misunderstood>?

[alexbelgium]

Hi, sorry i have never used the app updater and thought from this phrase in the readme that the newinstaller.sh script was the recommended way to update : The installer takes care of any and all necessary updates, so you can run that as the very first command upon the first boot, if you'd like.

I looked at the updater code and this line find $HOME/Bird* -type f ! -perm -g+wr -exec chmod g+wr {} + 2>/dev/null should give the same permissions to the newly added files

Sorry for the confusion - just meant it doesn't seem that a specific permission for this new script should be set

[lloydbayley]

Nono, it's fine. It's just that people won't run that or know to run it. They will use (hopefully) the internal update button etc etc. I think you're on the right track now. @Nachtzuster care to comment?