Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support configuration as script arguments #7

Merged
merged 9 commits into from
Aug 4, 2020

Conversation

hammady
Copy link
Contributor

@hammady hammady commented Jul 27, 2020

  • Support configuration as script arguments, for example --lat 20.23 --lng 50.552 --method Egypt. These settings get persisted automatically so that later runs do not have to receive any arguments
  • Set the volume of Azaan using arguments, for example: --fajr-azaan-volume 1200 and --azaan-volume 1800
  • Make the script portable by detecting the current directory
  • Remove files ignored already like *.pyc and adhan.log

Copy link
Owner

@achaudhry achaudhry left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Jazak Allah these are great enhancements. Thank you for taking the time to add these.

I left a few suggestions. One is a bug fix and the rest are minor updates. If you can make those updates that would be great. I'll merge once done.

Thanks again!

hammady and others added 4 commits July 29, 2020 22:02
Co-authored-by: Abrar Chaudhry <2893677+achaudhry@users.noreply.github.com>
Co-authored-by: Abrar Chaudhry <2893677+achaudhry@users.noreply.github.com>
@hammady hammady requested a review from achaudhry July 29, 2020 19:49
@hammady
Copy link
Contributor Author

hammady commented Jul 30, 2020

Jazak Allah these are great enhancements. Thank you for taking the time to add these.

I left a few suggestions. One is a bug fix and the rest are minor updates. If you can make those updates that would be great. I'll merge once done.

Thanks again!

Jazana Wa'eyakom, I have addressed all of your comments. Please let me know if anything more is needed to merge this.

@Zamy97
Copy link

Zamy97 commented Aug 2, 2020

I don't know if this would be the right spot to ask but let me know if not I'll remove this!

I am just playing with the praytimes.py and updateazantimer.py and instead of playing the audio on a pi, I am trying to run this in my local mac with the help of crontab but what is confusing me is how could I work around this command that is used for omxplayer to run the audio. omxplayer -o local /home/pi/adhan/Adhan-fajr.mp3 > /dev/null 2>&1 How could I work around that in order to be able to able to play the Adhan on my local machine?

Also, it seems as my jobs gets created on my local machine but the audio file never really plays.

Any tips or suggestions would be appreciated.

@hammady
Copy link
Contributor Author

hammady commented Aug 2, 2020

I don't know if this would be the right spot to ask but let me know if not I'll remove this!

I am just playing with the praytimes.py and updateazantimer.py and instead of playing the audio on a pi, I am trying to run this in my local mac with the help of crontab but what is confusing me is how could I work around this command that is used for omxplayer to run the audio. omxplayer -o local /home/pi/adhan/Adhan-fajr.mp3 > /dev/null 2>&1 How could I work around that in order to be able to able to play the Adhan on my local machine?

Also, it seems as my jobs gets created on my local machine but the audio file never really plays.

Any tips or suggestions would be appreciated.

@Zamy97 that's off topic, but my answer is that for macOS the preferred way to run timed jobs is launchd. Although cron is still supported, but it is not recommended, and I couldn't get it to run on my Mac.

@achaudhry
Copy link
Owner

Agreed with @hammady. @Zamy97 see my response on the issue you've created.

@hammady I'll look at the changes and merge them in tomorrow InshaAllah! Been busy with Eid and all :) Thanks for making the updates!

@hammady
Copy link
Contributor Author

hammady commented Aug 4, 2020

Agreed with @hammady. @Zamy97 see my response on the issue you've created.

@hammady I'll look at the changes and merge them in tomorrow InshaAllah! Been busy with Eid and all :) Thanks for making the updates!

@achaudhry let me know if you have questions so far to be able to merge.

@achaudhry
Copy link
Owner

@hammady sorry man, been extremely busy over the last few days but I haven't forgotten about this I promise :) Will look shortly. Thanks

@achaudhry achaudhry merged commit 3331c80 into achaudhry:master Aug 4, 2020
@hammady
Copy link
Contributor Author

hammady commented Aug 5, 2020

@hammady sorry man, been extremely busy over the last few days but I haven't forgotten about this I promise :) Will look shortly. Thanks

Thanks @achaudhry, now #10 :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants