More Piezo love

[BrianGenisio]

I saw that there is a Piezo PR in the making for re-working the song/play interface.

I was actually thinking of some more enhancements to Piezo. Before I get started, can someone tell me if I am on/off track?

1. Piezo.noTone (off) doesn't actually stop a tone. It just sets the pin to zero while the existing timeout setup will clobber that very shortly. I'd fix that.
2. Playing a tone() isn't terribly intuitive. I have to send in the half-period length in microseconds. I'd add a convenience function called frequency() that takes a frequency in htz which translates to the proper tone value.
3. I'd probably then refactor the song function (in progress) to use frequencies since that is what most musicians will relate to. This wouldn't affect the external interface.
4. There isn't any documentation for Piezo in the API documentation. I'd fix that.
5. Tests and examples supporting above.

Thoughts?

[julianduque]

Hello @BrianGenisio, I'm working on the Piezo improvements but I need to change the tempo to act like bpm, Piezo was hard to implement due to lack of microsecs timing in JavaScript, I'll finish the current PR this week and it would be awesome to receive your contributions :), will take this recommendations into account.

<3

[BrianGenisio]

Thanks, @julianduque. I'll hold off on making changes until you are done. I'm happy to contribute but I don't want to step on your toes :)

[julianduque]

@BrianGenisio better to work together, if you want take a look at the discussion in https://github.com/rwaldron/johnny-five/issues/372 and https://github.com/rwaldron/johnny-five/pull/386, i'm traveling right now but will working on this hopefully this weekend :)

[BrianGenisio]

Gotcha. The changes I am proposing are mostly orthogonal to your changes. I'll fork your repo and give you a PR that doesn't touch the song function at all and we can start from there. Once the frequency function is in place, it should be pretty clear how it will be used inside of play with minimal change from what you are working.

[Resseguie]

@BrianGenisio let me know if I can help test anything with this. I've been following and testing the other issues too.

[rwaldron]

:+1: :clap:

[rwaldron]

cc @lyzadanger

[lyzadanger]

Let me know when stuff is extant and I can help with examples and docs! Someone let me know where song ends up :) —I'm curious. @BrianGenisio I could help with API docs if you'd like!

[BrianGenisio]

I've started by getting noTimer() clearing the timer: https://github.com/BrianGenisio/johnny-five/commit/4c5799a66fa484e9769328e2e403228a94cd16c9.

I added a test for tone() asserting that the timer gets cleared after the timeout. But I can't figure out how to progress the clock. I can't use this.clock.tick() because Piezo uses hrtime. That checkin currently fails.

Does anyone know how to properly mock out the nanotimer for the Piezo class?

[BrianGenisio]

@lyzadanger That would be really great! :thumbsup:

[rwaldron]

@BrianGenisio try https://github.com/jhnns/rewire?

[gorhgorh]

Hi there, sorry to hop in the train with ideas that may not totally fit in, but still are related.

one of the main problem of node and the music is linked to the time management, most sound cards deal with that with an integrated quartz.

There should be a clean separation between the idea to activate and electronic component (ie buzz the servo) and play a tune.

I'm aware that hrtime is a huge improvement (it will definitely help to have a better match at the note frequencies), but still, imagine you want to make 2 arduinos playing together, linked, this could prove to be quite a chalenge, even with it.

maybe we should not only work with playing a tune with the piezo, but rather create a module that play tunes, on a piezo, or that activates solenoids etc etc, This module could also take midi input, allowing it to sync to other devices, and use the sound card's quartz to deal with tempo, and focus on the note accuracy on the piezo lib ?

MIDI is theres since a long time and is a sturdy protocol that can be passed from boards to board via RX/TX, and at some point, I'm sure that our little Piezo bots will want to communicate with others

since I'm working on an implementation of a stepper based band, i would totally love to be able to control a piezo along as well :)

what would you think of splitting the two feature so other modules can benefit from playing a sound, simplifies the piezo lib, and make our bots midi ones ?

2014-07-01 21:43 GMT+02:00 Rick Waldron notifications@github.com:

@BrianGenisio https://github.com/BrianGenisio try https://github.com/jhnns/rewire?

— Reply to this email directly or view it on GitHub https://github.com/rwaldron/johnny-five/issues/398#issuecomment-47701116 .

[BrianGenisio]

@rwaldron Good suggestion with rewire, but it doesn't seem to help. I can rewire process.hrtime() just fine. The problem is that nanotimer uses setImmediate. Unlike setTimeout or setInterval in which I can use this.clock.tick() to cause the callbacks to get called, setImmediate doesn't play nicely that way.

I think my next approach is to mock out the nanotimer itself. Perhaps I can use "rewire" for that.

[BrianGenisio]

Nevermind. I'm thinking about it all wrong. This test initializes the sinon useFakeTimers even though there is no good reason to do so. Everything is outside of anything that it can use.

Instead, I can test that the tone gets shut off after its time by adding a short setTimeout to handle the assertions. I had tried this before, but it mucked everything up because the timers were fake. Removing this did the trick.

https://github.com/BrianGenisio/johnny-five/commit/42f1434be7b842c9867234a354e2bee51e3c2e56

[BrianGenisio]

@gorhgorh I have been thinking a bit about what you are suggesting. I do wonder how well this existing mechanism for generating a square wave will be in practice. On my Arduino over USB, it seems like to work fine. But what about other physical transports, like WiFi. I should hook this up to my SparkCore, (whilst two laptops are streaming movies) and see what the tones sound like. My guess is that the tones will sound somewhat broken.

Does Firmata have a module system that we can probe the hardware if it supports a function? For instance, is it possible to create a square_wave extension and call it on the hardware via the Firmata link if it exists? Does Johnny-five even require that we talk to devices via Firmata? I don't know the answer to these questions, but invoking functions on the hardware sounds like a hard problem in the general case. Has it been solved in an effective way already?

If so, I could see how passing MIDI data to hardware starts to make sense as opposed to processing the MIDI on the host and sending the tones across as square waves.

[BrianGenisio]

Ok, I created a PR on Julian's fork to incorporate my changes.

[julianduque]

Thanks @BrianGenisio, great refactor, will review and test and the idea is to send our final PR this weekend :+1:

[lyzadanger]

As I work on docs for the combination of @julianduque and @BrianGenisio 's changes, I noticed a few items I wouldn't mind taking on if y'all think they're worthy:

• The thread for the original Piezo PR mentions changing tempo to be a BPM-like value instead of the current non-intuitive ms-based unit. I don't think anyone's picked that up. I could work on that.
• I feel like the values in the song attribute of the tune object for the play method could be more flexible, able to take frequency-based notes as well as those in Piezo.Notes, not require an array if the beat is the default, etc. Granted one would typically want to stick to the defined syntax (two-element arrays) for song notes for readability, but it would be a nice shorthand to be able to throw in the ocassional null without wrapping it in an array, e.g. I've actually already implemented and tested this on a branch derived from @julianduque and @BrianGenisio 's PRs.

I've started working on an API documentation page for the wiki, as well...Will stand back now and wait for PRs to land and settle out...

[rwaldron]

Once those changes are landed, you should dive into these

On Saturday, July 5, 2014, Lyza Gardner notifications@github.com wrote:

As I work on docs for the combination of @julianduque https://github.com/julianduque and @BrianGenisio https://github.com/BrianGenisio 's changes, I noticed a few items I wouldn't mind taking on if y'all think they're worthy:

• The thread for the original Piezo PR https://github.com/rwaldron/johnny-five/pull/386 mentions changing tempo to be a BPM-like value instead of the current non-intuitive ms-based unit. I don't think anyone's picked that up. I could work on that.
• I feel like the values in the song attribute of the tune object for the play method could be more flexible, able to take frequency-based notes as well as those in Piezo.Notes, not require an array if the beat is the default, etc. Granted one would typically want to stick to the defined syntax (two-element arrays) for song notes for readability, but it would be a nice shorthand to be able to throw in the ocassional null without wrapping it in an array, e.g. I've actually already implemented and tested this https://github.com/lyzadanger/johnny-five/commit/c990898ed4ddfebeb891b5670804488cc9210096 on a branch derived from @julianduque https://github.com/julianduque and @BrianGenisio https://github.com/BrianGenisio 's PRs.

I've started working on an API documentation page for the wiki, as well...Will stand back now and wait for PRs to land and settle out...

— Reply to this email directly or view it on GitHub https://github.com/rwaldron/johnny-five/issues/398#issuecomment-48099128 .

[lyzadanger]

@rwaldron Do we care about backward compatibility for tempo? OK to change the behavior on that (not backward compatible) or should I use a new attribute and support both? My guess is that "we're young enough" to change the behavior, but thought I'd check.

[rwaldron]

I'm open to breaking back compat for Piezo since its not documented and was mostly avoided for 2 years :)

[BrianGenisio]

Ok, cool. So as far as I understand, @lyzadanger is tackling the tempo and play API mods and finishing up the docs. I'll stick around on this thread to see if there is anything else left to do from my perspective, but it looks like we are just about done. Thanks y'all! Looking forward to using the new API.

[julianduque]

Awesome, was without Internet this weekend but will publish the songs module today, @lyzadanger let me know how the api changes to handle tempo with your improvements to adapt the songs :)

[rwaldron]

I'm really proud of this collaborative effort—you three have done exceptional work :)

[lyzadanger]

Waitin' on some of these open PRs to get merged and then I'll open another PR with my tempo/API changes and get the updated API docs live.

While I'm in there...anyone mind if I add an optional done callback arg to play (mirroring the metaphor in Led, e.g.)? In my testing I found it's awfully easy to play songs "on top of" each other.

[BrianGenisio]

Great idea. :+1:

[rwaldron]

Are we waiting for @julianduque to merge @BrianGenisio updates? Or do I need to land something?

[julianduque]

@BrianGenisio PR is merged, waiting on @lyzadanger tempo fix

[lyzadanger]

Ah, cool. I’ll try to find some time tonight to get updated to your latest and then cherry-pick my API flexibility, tempo and callback commits onto it. Then I’ll open a PR. It could be tomorrow before I can do this, but I hope today!

On Jul 7, 2014, at 9:51 PM, Julian Duque notifications@github.com wrote:

@BrianGenisio PR is merged, waiting on @lyzadanger tempo fix

— Reply to this email directly or view it on GitHub.

[Resseguie]

@rwaldron I believe all this was taken care of in the piezo merge and can now be closed?

[rwaldron]

Yes