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

"Await"ing JZZ fails to enumerate MIDI devices #22

Closed
justcfx2u opened this issue Oct 13, 2019 · 19 comments
Closed

"Await"ing JZZ fails to enumerate MIDI devices #22

justcfx2u opened this issue Oct 13, 2019 · 19 comments

Comments

@justcfx2u
Copy link
Contributor

I tried to use JZZ in TypeScript by the documentation,
Trying to call JZZ() yields This expression is not callable.

import * as JZZ from 'jzz';

Experimentally, I changed the import to the following:

import JZZ from 'jzz';

// ...

let x = await JZZ().or('oh no!').and('Everything is good!');

Written like this, I am able to use JZZ().

However, x.info().outputs and x.info().inputs are empty even though "Everything is good!" is logged into the console. It seems that there are some internal errors inside JZZ.

screenshot 1

The above failure was caused while in useEffect of a React functional component.

const App: React.FC = () => {
  useEffect(() => {
    async function initJZZ() {
      let x = await JZZ().or('oh no!').and('Everything is good!');
      console.dir(x.info());
      console.dir(x);
    }
    initJZZ();
  }, []);

  return (<p>JSX stuff here</p>);
}

However, referencing issue #6, I put in some raw WebMIDI code in useEffect() and it seems to have worked.

  useEffect(() => {
    let webmidi:any;

    function refresh() {
      console.log(["wew", webmidi.inputs, webmidi.outputs]);

      console.log('---------');
      console.log('OUTPUTS');
      webmidi.outputs.forEach(function(port:any) {
        console.log('    ' + port.name +
          '; manufacturer: ' + port.manufacturer +
          '; version: ' + port.version +
          '; state: ' + port.state);
      });
      console.log('---------');
      console.log('INPUTS');
      webmidi.inputs.forEach(function(port:any) {
        console.log('    ' + port.name +
          '; manufacturer: ' + port.manufacturer +
          '; version: ' + port.version +
          '; state: ' + port.state);
      });

    }
    function fail(err:any) {
      var s = 'Cannot start WebMIDI';
      if (err) s += ': ' + err;
      console.log(s);
    }
    
    function success(midiaccess:any) {
      webmidi = midiaccess;
      // webmidi.onstatechange = refresh;
      refresh();
    }
    
    try {
      navigator.requestMIDIAccess().then(success, fail);
    }
    catch (err) {
      console.log('Cannot start WebMIDI: ' + err);
    }
  }, []);

screenshot 2

I can only assume that something in JZZ doesn't like TypeScript or how it's referenced within React.

The end goal is either a PWA (Progressive Web Application) hosted on Firebase and/or an Electron app written in React to configure a specific MIDI device using SysEx.

@jazz-soft
Copy link
Owner

Both TypeScript and React are not my strongest skills, I would appreciate some help on those...

@jazz-soft
Copy link
Owner

JZZ is written in pure JavaScript.
There is a TypeScript definition file named "index.d.ts" that is supposed to closely match the JZZ functionality. Possibly, it needs improvements.
You can delete this file and have TypeScript treat all JZZ-related stuff as "any", or you can suggest some corrections.

Regarding the error in the _log array - you can ignore those: JZZ tries to initialize various MIDI engines before it finds the one working on the current system.

@justcfx2u
Copy link
Contributor Author

This is my first project using TypeScript. :)

I will see if I can figure it out as well.

@jazz-soft
Copy link
Owner

I have just updated dependencies in https://www.npmjs.com/package/jazz-midi-electron
Please try if it works any better now.
And I have double-checked that import * as JZZ from 'jzz'; works correctly (but did not try it with React)

@justcfx2u
Copy link
Contributor Author

I'm not sure this will help. Will it? At the moment I'm just using 'jzz' in my React project without jazz-midi-electron.

@jazz-soft
Copy link
Owner

I guess you have the latest JZZ version then...
Is your code in open access? Perhaps, I could look at it...

@justcfx2u
Copy link
Contributor Author

The pull request is NOT good. I am not completely sure why it works, and it could be written much more cleanly. However, it's still worth taking a look.

This does not seem to be a TypeScript issue, but a general one about the way JZZ is being accessed somehow.

I have not tested for regressions.

My research led me to believe that the call is somehow losing "context", likely due to the global variable in some way.

I wish I knew why this started returning results with this change.

@justcfx2u
Copy link
Contributor Author

I encourage rejection of the pull request, I made it mostly as a starting point for further research, but I am likely to proceed with my project with the change as is.

@justcfx2u
Copy link
Contributor Author

justcfx2u commented Oct 16, 2019

I still need to access JZZ using the following:

  // useEffect(() => {
  // ...
    async function initJZZ() {
        let x = await JZZ.default({sysex:true}).or('oh no!').and('Everything is good!'); 
        console.dir(x.info());
    }
    initJZZ();
  // ...
  // }, []);

This probably isn't a good design pattern for a number of reasons but I was just focused on getting MIDI devices enumerated.

@jazz-soft
Copy link
Owner

By the way, on which system/browser did you see the "navigator" error?
It's strange that I have not spot it before...

@justcfx2u
Copy link
Contributor Author

I have created a minimal test-case in React and "pure JavaScript".

Basic Repro

  1. npx create-react-app jzz-repro
  2. cd jzz-repro
  3. npm install --save jzz
  4. Edit src/App.js

Make code look like this:

import React, { useEffect } from 'react';
import logo from './logo.svg';
import './App.css';
import * as JZZ from 'jzz';

function App() {

  useEffect(() => {
    async function initJZZ() {
      let x = await JZZ().or("fail").and("ok");
      console.log(x.info());
    }
    initJZZ();
  }, []);

  return (
    // ...
  1. yarn start
  2. Note that no MIDI devices are enumerated in the browser console from the output of the console.log

Apply Fix

  1. Apply patch to node_modules/jzz/javascript/JZZ.js, save.
  2. Ctrl-C to stop yarn server, then yarn start again
  3. When page loads in browser again, MIDI devices are enumerated successfully.

Tested on Win10 1903 with Chrome Version 77.0.3865.120 (Official Build) (64-bit).

@justcfx2u justcfx2u changed the title React + TypeScript failure JZZ + React fails to enumerate MIDI devices Oct 16, 2019
@justcfx2u
Copy link
Contributor Author

@jazz-soft
Copy link
Owner

And which patch do you mean? #23 or something else?

@justcfx2u
Copy link
Contributor Author

Yes, 23.

@justcfx2u
Copy link
Contributor Author

Made an even smaller standalone test.

<!DOCTYPE html>
<html>
<head>
<meta charset="UTF-8">
<title>JZZ Ultra-mini test</title>
</head>
<body>
<h1>JZZ Ultra-mini test</h1>

<p>Hello.</p>

</body>
<script src="./JZZ.js"></script>
<script>
async function initJZZ() {
  let j = await JZZ();
  console.log(j.info());
}

initJZZ();

</script>
</html>

Does not require React or a webserver. Simply open it in a browser. Same symptom/problem, same patch fixes it.

@justcfx2u justcfx2u changed the title JZZ + React fails to enumerate MIDI devices "Await"ing JZZ fails to enumerate MIDI devices Oct 17, 2019
@justcfx2u
Copy link
Contributor Author

justcfx2u commented Oct 17, 2019

Using a promise (as shown below) works fine, but having the ability to use "await" (above) is definitely a bonus.

EDIT: This also doesn't work unless the patch is applied.

<!DOCTYPE html>
<html>
<head>
<meta charset="UTF-8">
<title>JZZ Ultra-mini test</title>
</head>
<body>
<h1>JZZ Ultra-mini test</h1>

<p>Hello.</p>

</body>
<script src="./JZZ.js"></script>
<script>
let j = JZZ();
j.then(() => {console.log(j.info())});

</script>
</html>

@justcfx2u
Copy link
Contributor Author

The Pull Request #24 seems to be a better patch.

@justcfx2u
Copy link
Contributor Author

Thank you. With any luck this change didn't break anyone else's project, but mine is going very well now.

@jazz-soft
Copy link
Owner

Thanks a lot for a very helpful discussion!
I'll release a new version after a bit more testing.

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

No branches or pull requests

2 participants