-
-
Notifications
You must be signed in to change notification settings - Fork 7k
RTSP monitor #5954
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
base: master
Are you sure you want to change the base?
RTSP monitor #5954
Conversation
CommanderStorm
left a comment
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.
Nice work. 👍🏻
Could you add a testcase to ensure that there are no regressions with this monitoring type?
Doing so greatly increases the stability of this montior by preventing for example bad depenecy bumps, ... breaking you.
src/pages/EditMonitor.vue
Outdated
|
|
||
| <template v-if="monitor.type === 'rtsp'"> | ||
| <div class="my-3"> | ||
| <label for="rtspUsername" class="form-label">RTSP {{ $t("Username") }}</label> |
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.
please translate always with full translation keys.
=> "RTSP-Username" instead of excluding RTSP.
There are Left to right and right to left languages and we don't want to cause our translators more work in this department
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.
@CommanderStorm, I was intentionally trying to exclude "RTSP" from the label — similar to how the MQTT type works. The username and other keywords are already using translations.
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.
that was a mistake for MQTT then which was missed.
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.
hey @CommanderStorm can you guide me how to use weblate to add a new word
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.
Shure.
You just add every translation that should become avaliable on weblate to en.json and once merged, our awsome army of translators can translate this.
Important: only add things to en.json, all other translations have to go via weblate to prevent merge conflicts.
src/pages/EditMonitor.vue
Outdated
| </div> | ||
| <div class="my-3"> | ||
| <label for="rtspPath" class="form-label">RTSP {{ $t("path") }}</label> | ||
| <input id="rtspPath" v-model="monitor.rtspPath" type="text" class="form-control"> |
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.
do these have a typical format? if yes, could you add a placeholder or helptext?
What a path is, might otherwise not be very self-explainatory without extra docs.
|
@CommanderStorm any other changes are required ? |
|
I am going to review it once you are done with the changes that I requested originally. There are a few open things, such as the database layout or HiddenInput. |
❗ Important Announcement
Click here for more details:
🚧 Temporary Delay in Feature Requests and Pull Request Reviews
At this time, we may be slower to respond to new feature requests and review pull requests. Existing requests and PRs will remain in the backlog but may not be prioritized immediately.
We appreciate your patience and understanding as we continue to improve Uptime Kuma.
🚫 Please Avoid Unnecessary Pinging of Maintainers
We kindly ask you to refrain from pinging maintainers unless absolutely necessary. Pings are reserved for critical/urgent pull requests that require immediate attention.
Why: Reserving pings for urgent matters ensures maintainers can prioritize critical tasks effectively.
📋 Overview
What problem does this pull request address?
Uptime Kuma currently lacks support for RTSP (Real Time Streaming Protocol) monitoring, which is important for users monitoring IP cameras and similar devices.
What features or functionality does this pull request introduce or enhance?
This PR introduces RTSP-related fields to the
monitortable to store credentials and stream path, allowing RTSP monitors to be configured in the future:rtsp_usernamertsp_passwordrtsp_path🔗 Related Issues
🔄 Changes
🛠️ Type of change
📄 Checklist
hostname:"807e9439d5ca.entrypoint.cloud.wowza.com",
port:"1935",
rtsp_username: "807e9439d5ca",
rtsp_password: "",
rtsp_path: "/app-rC94792j/068b9c9a_stream2",
use this smaple details to check
📷 Screenshots or Visual Changes
rtsp_username,rtsp_password,rtsp_path