-
-
Notifications
You must be signed in to change notification settings - Fork 7
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
Fixes/more controls #52
Fixes/more controls #52
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry, but these changes would make the plugin unusable by other users as-is.
pan_relative: { use: ko.observable(false), value: ko.observable(undefined), min: ko.observable(-4480), max: ko.observable(4480), step: ko.observable(0) }, | ||
pan_speed: { use: ko.observable(false), value: ko.observable(undefined), min: ko.observable(0), max: ko.observable(100), step: ko.observable(1) }, | ||
tilt_absolute: { use: ko.observable(false), value: ko.observable(undefined), min: ko.observable(0), max: ko.observable(100), step: ko.observable(1) }, | ||
tilt_relative: { use: ko.observable(false), value: ko.observable(undefined), min: ko.observable(0), max: ko.observable(100), step: ko.observable(1) }, | ||
tilt_relative: { use: ko.observable(false), value: ko.observable(undefined), min: ko.observable(-1920), max: ko.observable(1920), step: ko.observable(0) }, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Min, max and step should never be hard coded here.
They are values reported by the camera and different cameras have different values. If you've got a camera that is misbehaving by reporting bad values then a different approach would be needed, such as what I've proposed in #51
save_user_settings: { use: ko.observable(false) }, | ||
restore_user_settings: { use: ko.observable(false) }, | ||
restore_factory_settings: { use: ko.observable(false) }, | ||
pan_reset: { use: ko.observable(false) }, | ||
tilt_reset: { use: ko.observable(false) }, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If these are deleted then users with cameras that report these controls will have errors.
Also, if you want to contribute, please target the rc branch. |
Sorry I meant to remove the hard coded lines. I'll update.
On the second point, you don't have those in RC. My changes match RC.
I'll work from RC going forward but keep it separate until I'm done.
…On Tue, Aug 31, 2021, 6:01 AM Taylor Talkington ***@***.***> wrote:
***@***.**** requested changes on this pull request.
Sorry, but these changes would make the plugin unusable by other users
as-is.
------------------------------
In octoprint_CameraSettings/static/js/CameraSettings.js
<#52 (comment)>
:
> + pan_relative: { use: ko.observable(false), value: ko.observable(undefined), min: ko.observable(-4480), max: ko.observable(4480), step: ko.observable(0) },
pan_speed: { use: ko.observable(false), value: ko.observable(undefined), min: ko.observable(0), max: ko.observable(100), step: ko.observable(1) },
tilt_absolute: { use: ko.observable(false), value: ko.observable(undefined), min: ko.observable(0), max: ko.observable(100), step: ko.observable(1) },
- tilt_relative: { use: ko.observable(false), value: ko.observable(undefined), min: ko.observable(0), max: ko.observable(100), step: ko.observable(1) },
+ tilt_relative: { use: ko.observable(false), value: ko.observable(undefined), min: ko.observable(-1920), max: ko.observable(1920), step: ko.observable(0) },
Min, max and step should never be hard coded here.
They are values reported by the camera and different cameras have
different values. If you've got a camera that is misbehaving by reporting
bad values then a different approach would be needed, such as what I've
proposed in #51
<#51>
------------------------------
In octoprint_CameraSettings/static/js/CameraSettings.js
<#52 (comment)>
:
> - save_user_settings: { use: ko.observable(false) },
- restore_user_settings: { use: ko.observable(false) },
- restore_factory_settings: { use: ko.observable(false) },
- pan_reset: { use: ko.observable(false) },
- tilt_reset: { use: ko.observable(false) },
If these are deleted then users with cameras that report these controls
will have errors.
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#52 (review)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/ACPDJYJX7YKMZPRJYJW2V7TT7SZA7ANCNFSM5DDJQPCA>
.
Triage notifications on the go with GitHub Mobile for iOS
<https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675>
or Android
<https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub>.
|
This works for pan/tilt but needs buttons. Will do that soon.