Skip to content

Feature: adding reel burning detection#47

Merged
dereklee0310 merged 8 commits intodereklee0310:mainfrom
pwnyprod:feature/adding-reel-burning-detection
Feb 24, 2025
Merged

Feature: adding reel burning detection#47
dereklee0310 merged 8 commits intodereklee0310:mainfrom
pwnyprod:feature/adding-reel-burning-detection

Conversation

@pwnyprod
Copy link
Copy Markdown
Contributor

lookup commit messages for further insights.

…r prompts

🎨 style(app): correct settings title typo in display settings
🔧 fix(monitor): implement reel burning detection with new method
🔧 fix(setting): add reel burning icon position to settings
🧹 chore(.gitignore): ignore IDE specific files by adding .idea/
📦 upgrade(requirements): update prettytable version to 3.14.0
…etection

🎨 style(src/setting): adjust reel burning icon position for better alignment
🔧 refactor(src/monitor): update is_reel_burning method to return boolean instead of None
Copy link
Copy Markdown
Owner

@dereklee0310 dereklee0310 left a comment

Choose a reason for hiding this comment

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

Hi

Thanks for the pull request.
I have a few suggestions that need to be addressed:

  1. The link in app.py should remain unchanged.
  2. Please add the burning reel image to the folders under static/ for other languages.
  3. Using start.bat to launch the script is an interesting approach, but it seems cumbersome to answer every prompt when only a few arguments are needed. I don't think we have to add it.

@pwnyprod
Copy link
Copy Markdown
Contributor Author

Hi

Thanks for the pull request. I have a few suggestions that need to be addressed:

  1. The link in app.py should remain unchanged.
  2. Please add the burning reel image to the folders under static/ for other languages.
  3. Using start.bat to launch the script is an interesting approach, but it seems cumbersome to answer every prompt when only a few arguments are needed. I don't think we have to add it.
  1. fixed, sorry was a test
  2. this was a test for image recognition
  3. excluded, i use this for quick changing between the options i need for different profiles

@dereklee0310
Copy link
Copy Markdown
Owner

LGTM
One last thing, have you tested it out locally?
Just want to make sure it works well since I don't plan on testing it myself.

@pwnyprod
Copy link
Copy Markdown
Contributor Author

LGTM One last thing, have you tested it out locally? Just want to make sure it works well since I don't plan on testing it myself.

yes im using it right now, i changed it to a tolerance value because it got problems detecting in the night. i still got some "overreactions" when the hand is moving under the symbol while pulling.
But i think this is okay, its more like a sensitive behaviour.

@dereklee0310
Copy link
Copy Markdown
Owner

Correct me if I'm wrong, but according to the tests I've done, seems like the color of the icons is not affected by lighting?

@pwnyprod
Copy link
Copy Markdown
Contributor Author

pwnyprod commented Feb 23, 2025

Correct me if I'm wrong, but according to the tests I've done, seems like the color of the icons is not affected by lighting?

you was right, the "flickering" i assumed because of the transparency was because i got a wrong pixel position in the window. i corrected this and tested it with a feeder. it works like a charm now.

@dereklee0310 dereklee0310 merged commit 626954a into dereklee0310:main Feb 24, 2025
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.

2 participants