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

Add pio/ir_nec #129

Merged
merged 4 commits into from
Oct 26, 2021
Merged

Add pio/ir_nec #129

merged 4 commits into from
Oct 26, 2021

Conversation

mjcross
Copy link
Contributor

@mjcross mjcross commented Jun 26, 2021

As discussed with @lurch and referenced by @aallan this PR adds a PIO example to demonstrate sending and receiving infra-red codes in 'NEC' format: pio/ir_nec.

There are three sub-directories: the loopback example, and a library each for the transmit and receive functions. As requested I have included a README.adoc and wiring diagram along the lines of gpio/hello_7segment

I've left my name in the copyright message with the BSD-3Clause licence but I'm OK for you to change that if necessary. Incidentally I have followed the format of the other examples by adding the example_auto_set_url() stuff into CMakeLists.txt but I'm afraid I must admit I don't really know what I'm doing with that.

@lurch
Copy link
Contributor

lurch commented Jun 28, 2021

adding the example_auto_set_url() stuff into CMakeLists.txt but I'm afraid I must admit I don't really know what I'm doing with that.

IIRC it adds a URL string to the .uf2 file that picotool info can display.

pio/ir_nec/README.adoc Outdated Show resolved Hide resolved
@lurch
Copy link
Contributor

lurch commented Jun 28, 2021

I wonder if your wiring diagram might be clearer if you used a black wire for GND and a red wire for 3V3? 🙂

pio/ir_nec/README.adoc Outdated Show resolved Hide resolved
@mjcross
Copy link
Contributor Author

mjcross commented Jun 28, 2021

I wonder if your wiring diagram might be clearer if you used a black wire for GND and a red wire for 3V3? 🙂

Yes I think that's a very good idea - will do

- README.adoc
Clarify that the wavelength of the IR LED should be compatible with the
detector used;
Correct the number of jumper wires in the BOM.

- Wiring diagram
Use red wire for +3v3 and black for GND.

- Fritzing source file
Fix having added the wrong file.

- ir_loopback/ir_loopback.c
Correct typo in comment.

- nec_receive_library/nec_receive.pio
Amend comment to clarify that the provided 'init' function sets the
state machine clock divider relative to the system clock.
Amend comment to clarify that the 'init' function sets up the ISR as
described.
@mjcross
Copy link
Contributor Author

mjcross commented Jun 28, 2021

I wonder if your wiring diagram might be clearer if you used a black wire for GND and a red wire for 3V3? 🙂

Yes I think that's a very good idea - will do

Fixed by 0162c73

@mjcross
Copy link
Contributor Author

mjcross commented Jun 28, 2021

adding the example_auto_set_url() stuff into CMakeLists.txt but I'm afraid I must admit I don't really know what I'm doing with that.

IIRC it adds a URL string to the .uf2 file that picotool info can display.

Ah, thanks - useful to know

@mjcross mjcross closed this Jun 28, 2021
@mjcross mjcross reopened this Jun 28, 2021
@mjcross
Copy link
Contributor Author

mjcross commented Jun 28, 2021

oops!

@mjcross
Copy link
Contributor Author

mjcross commented Jul 11, 2021

bump

@mjcross
Copy link
Contributor Author

mjcross commented Jul 31, 2021

bump again

@mjcross
Copy link
Contributor Author

mjcross commented Aug 10, 2021

@lurch is this going anywhere please?

@lurch
Copy link
Contributor

lurch commented Aug 10, 2021

It's out of my hands - it's up to @kilograham if he wants to take this any further... I think he's just very busy?

@mjcross
Copy link
Contributor Author

mjcross commented Sep 27, 2021

@kilograham - please advise if this is of any interest, if not I will delete it

@kilograham
Copy link
Contributor

Sorry, yes we are doing an example update now.

@ghollingworth, @aallan is the license OK or do we need to add RPTL too?

Copy link
Contributor

@liamfraser liamfraser left a comment

Choose a reason for hiding this comment

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

Looks decent quality to me

@liamfraser
Copy link
Contributor

(Assuming the license situation is good)

@aallan
Copy link

aallan commented Oct 25, 2021

@ghollingworth, @aallan is the license OK or do we need to add RPTL too?

From my point of view, we're just fine with the original contributor retaining copyright so long as the license terms are clear and the code is submitted under a BSD 3 Clause. We don't necessarily demand (and haven't in the past) assignment of copyright for contributions that weren't work-for-hire. See this answer from @LizUpton on a similar topic in the documentation repo, raspberrypi/documentation#2079 (comment).

Copy link

@aallan aallan left a comment

Choose a reason for hiding this comment

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

In other words, LGTM.

@mjcross
Copy link
Contributor Author

mjcross commented Oct 25, 2021

Thanks all

@kilograham
Copy link
Contributor

Can you add a link to the top level README.md for completeness

@kilograham kilograham merged commit 1ec5e53 into raspberrypi:develop Oct 26, 2021
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.

5 participants