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.

Sign up for GitHub

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

Jump to bottom

JSON-IR : ADD Osram 24keys #244

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

JSON-IR : ADD Osram 24keys #244

wants to merge 3 commits into from

Conversation

kepon85
Copy link

@kepon85 kepon85 commented Jan 23, 2025

My 24 button ORAM remote control did not work with the proposed JSON, here is mine that works.

Thanks for WLEDL

@netmindz netmindz requested a review from blazoncek January 25, 2025 08:33
@netmindz
Copy link
Collaborator

I've not handled one of these before @blazoncek

Is this what you would expect from such a PR?

@kepon85
Copy link
Author

kepon85 commented Jan 25, 2025

I've not handled one of these before @blazoncek

Is this what you would expect from such a PR?

This is just a contribution to the doc... Is there another way to do this than a PR?

@blazoncek
Copy link
Collaborator

@netmindz I think it is ok, just links need to be checked that they do not lead to a malicious site.

netmindz
netmindz reviewed Jan 26, 2025
View reviewed changes
docs/interfaces/json-ir/json_infrared.md Outdated
@@ -55,6 +55,10 @@ When purchasing a remote, be sure that it is an infrared (IR) remote and not a r
24-key grey
[ir.json](24-key-grey_ir.json)

[![24-key-osram remote](24-key-osram.png)](https://www.aliexpress.com/item/4000704528801.html)
Copy link
Collaborator

Choose a reason for hiding this comment

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

This link gives me a not found

Copy link
Author

Choose a reason for hiding this comment

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

Sorry I deleted the link (which was active when I put it...)

@kepon85
Update json_infrared.md
3230d39 
Change 404 link aliexpress
@kepon85
Update json_infrared.md
e60fd01 
Delete Link  osram
kepon85
kepon85 commented Jan 26, 2025
View reviewed changes
Copy link
Author

@kepon85 kepon85 left a comment

Choose a reason for hiding this comment

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

Delete shop link

@blazoncek
Copy link
Collaborator

Why not just add another link to JSON file under regular 24-key remote?
As it is virtually identical to regular 24-key remote you can skip adding image and only provide JSON file IMO.

@kepon85
Copy link
Author

kepon85 commented Jan 26, 2025

Why not just add another link to JSON file under regular 24-key remote? As it is virtually identical to regular 24-key remote you can skip adding image and only provide JSON file IMO.

I think it's very good like that, if you want to change to do "as you please" it's possible...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@netmindz netmindz netmindz left review comments

@blazoncek blazoncek Awaiting requested review from blazoncek

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@kepon85 @netmindz @blazoncek