search

ozdemirburak / morse-code-translator

Translate text to Morse code and vice versa, with the option to play Morse code audio.
https://morsecodetranslator.com
MIT License
216 stars 57 forks source link

Feature/wpm option #28

Closed chris--jones closed 1 year ago

chris--jones commented 1 year ago

Resolves #27 Adding WPM and Volume options.

Note: This PR also includes updates to latest npm packages and some typescript recommended fixes.

Implementation:

For legacy support reasons the wpm is an optional field so that existing usage does not break, if defined it will set the unit and fwUnit properties to the same unit value as determined by the formula 60 / 50 * wpm - the formula details and reference I used may be viewed here: https://morsecode.world/international/timing.html

In order to make the implementation consistent with online examples the gaps between characters and words the unit of silence is now added conditionally and before the tone, provided we are not at the start or following a gap. This change could technically be considered breaking as it affects the audio for all timing methods, however the change only affects gaps in a very minor way and does not interfere with being able to decode a message.

Volume is implemented simply by tweaking the gain of the audio source by the provided percentage value, again defaulting to 100 to not break current implementations.

Finally the readme is updated to mention the new options.

ozdemirburak commented 1 year ago

Hello @chris--jones. Looks perfect, thanks.

I'll release v4.0.0 after this. So, legacy support is not needed. What do you think? Maybe we can make WPM not optional?

Also, I'd like to thank you for your great contributions. If you have a personal page or something, I'd be happy to link to it from https://morsedecoder.com/about/ or from README.md, which is up to you.

Thanks a lot.

chris--jones commented 1 year ago

I'll release v4.0.0 after this. So, legacy support is not needed. What do you think? Maybe we can make WPM not optional?

I don't know how other users would feel and I think the Farnsworth approach allows control over gaps which may help people learning morse code. I personally wouldn't introduce breaking changes without good reason. From a library maintenance perspective the complexity of supporting both approaches is minimal. It would be slightly simpler to have a single unit, but not enough to lose a potentially useful option in my opinion.

Having said all that, it's your library and I'm happy to remove the extra code (would be easy to put back if there was public outcry)

ozdemirburak commented 1 year ago

Thanks a lot, @chris--jones. Let's keep them then, your argument is fair enough.