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

Added support for Protelevision DVB-T Transmitter #9648

Merged
merged 32 commits into from Feb 19, 2019

Conversation

Projects
None yet
3 participants
@jozefrebjak
Copy link
Contributor

commented Jan 11, 2019

DO NOT DELETE THIS TEXT

Please note

Please read this information carefully. You can run ./scripts/pre-commit.php to check your code before submitting.

Testers

If you would like to test this pull request then please run: ./scripts/github-apply <pr_id>, i.e ./scripts/github-apply 5926
After you are done testing, you can remove the changes with ./scripts/github-remove. If there are schema changes, you can ask on discord how to revert.

I added os protelevision-t1 because i will add another os protelevision-t2, because when i switched to T2 then there is diferent sysObjectID and different sensors with different OIDs

jozefrebjak added some commits Jan 11, 2019

@murrant

This comment has been minimized.

Copy link
Member

commented Jan 11, 2019

@jozefrebjak looks like you forgot to add includes/poller/os/protelevision-t1.inc.php.
Are you using the github webui to create these? It is a lot easer to use the git cli once you learn how to use it :)

@murrant murrant added the Device 🖥 label Jan 11, 2019

@jozefrebjak

This comment has been minimized.

Copy link
Contributor Author

commented Jan 11, 2019

@murrant Yes, I forgot and yes I am adding it with web. I know i must to learn, how use cli. I installed a testing VM for librenms, and finnaly i can run ./scripts/save-test-data.php -o example-os. In my production install i can't and i don't know why

@murrant

This comment has been minimized.

Copy link
Member

commented Jan 12, 2019

Yeah, git has an initial hump to get over :)

@murrant

This comment has been minimized.

Copy link
Member

commented Jan 12, 2019

Looks good but the "icon" is extremely wide. Is there something more square you can use? You can move the current image to a logo. (check the docs for details)

jozefrebjak added some commits Jan 13, 2019

@murrant
Copy link
Member

left a comment

Looks good to me. You may want to update to take advantage of sensor grouping.

Test data will need to be updated (merge upstream first)

jozefrebjak added some commits Jan 17, 2019

@jozefrebjak jozefrebjak changed the title WIP Added support for Protelevision DVB-T Transmitter Added support for Protelevision DVB-T Transmitter Jan 17, 2019

@jozefrebjak

This comment has been minimized.

Copy link
Contributor Author

commented Jan 17, 2019

@murrant so i made some tuning, how that group sensors works ?

@murrant

This comment has been minimized.

Copy link
Member

commented Jan 17, 2019

@jozefrebjak an option way to visually group sensors in the webui. You might want to use it for your state sensors here actually.

@murrant

This comment has been minimized.

Copy link
Member

commented Jan 19, 2019

Screenshot here: #9606

jozefrebjak added some commits Jan 25, 2019

@jozefrebjak

This comment has been minimized.

Copy link
Contributor Author

commented Jan 25, 2019

@murrant I am happy with it, you can merge when it will be everything ok.

jozefrebjak added some commits Feb 5, 2019

@PipoCanaja

This comment has been minimized.

Copy link
Contributor

commented Feb 6, 2019

Hi @jozefrebjak
After a very quick look, it seems that your groups are not correctly set in the test file. You should probably regenerate them in order to pass the build tests. Basically, Travis must end up green ;)
PipoCanaja

murrant added some commits Feb 7, 2019

@murrant

murrant approved these changes Feb 7, 2019

@jozefrebjak

This comment has been minimized.

Copy link
Contributor Author

commented Feb 18, 2019

@murrant when will be possible to merge this ?

@murrant

This comment has been minimized.

Copy link
Member

commented Feb 19, 2019

Sorry, @jozefrebjak sometimes I don't get back to a PR after waiting for tests to finish.

Thanks for the PR 😃

@murrant murrant closed this Feb 19, 2019

@murrant murrant reopened this Feb 19, 2019

@murrant murrant merged commit 2eac72d into librenms:master Feb 19, 2019

4 of 6 checks passed

Travis CI - Pull Request Build Errored
Details
codeclimate 2 issues to fix
Details
Inspection Summary
Details
Node: analysis
Details
WIP Ready for review
Details
license/cla Contributor License Agreement is signed.
Details

@murrant murrant removed the User-Pending label Feb 19, 2019

@lock lock bot locked as resolved and limited conversation to collaborators Apr 20, 2019

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
You can’t perform that action at this time.