qmk / qmk_flasher

flashing utility for the Atmel AVR USB family
MIT License
63 stars 24 forks source link

Mac Version 0.5.0 and 0.3: Mouse pointer freezes every second and is extreeeeemly sensitive #32

Closed chief-wrench closed 7 years ago

chief-wrench commented 7 years ago

Also a debug screen of sorts opens, the pink underlayed "disabled" flashes every second bildschirmfoto 2017-01-15 um 22 56 35

NoahAndrews commented 7 years ago

We'll be releasing version 0.6 once we get a new icon, which will remove the developer tools window. Since you have it though, we can put it to good use. Is there any output on the Console tab?

NoahAndrews commented 7 years ago

I still want to hear what shows up in the Console tab of the developer tools window in the current version, but I also want to know if the latest version of the code has this issue for you. Try this: https://docs.google.com/uc?id=0B1MSp8fmGrzqbXI0YUV1Z3lMR0k&export=download

NoahAndrews commented 7 years ago

The flashing behavior you described in the developer window is expected behavior. Are you saying that every time it flashes, the mouse freezes briefly? By the sensitivity part, do you mean that the mouse cursor is moving faster/further than normal? Is that part constant while the app is running?

chief-wrench commented 7 years ago

@NoahAndrews That latest version fixed it - almost.

I re-started 0.3 to check for your answers:

By sensivity I mean the pointer is MUCH faster than usual, factor 10 maybe? The behaviour is constant and stops immediately on quitting the app.

NoahAndrews commented 7 years ago

Good to hear the sensitivity issue is gone. I can't imagine what could have been causing that.

Here's what's going on with the hanging. Whenever we check to see for a flashable keyboard, your pointer freezes. The latest version just checks less often, which I believe may have actually been an unintentional change. I had no idea the way we were checking for the board could cause such a huge, unacceptable performance issue. This is especially problematic, because we want to support more keyboards, and we'll have to run that check for each type of chip.

My guess is that you're using a Mac with a traditional hard drive instead of an SSD. Can you tell me what Mac you're using, what year it came out, the version of macOS it's running, and its full specs?

---- On Mon, 16 Jan 2017 01:45:53 -0500 notifications@github.com wrote ----

@NoahAndrews That latest version fixed it - almost.

The pointer has the usual sensivity. The pointer still hangs: about every four seconds, for about half a second. This is a huge improvement. I re-started 0.3 to check for your answers:

The console tab shows no output. The flashing is in sync with the pointer stutters. By sensivity I mean the pointer is MUCH faster than usual, factor 10 maybe? The behaviour is constant and stops immediately on quitting the app.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or mute the thread.

chief-wrench commented 7 years ago

Mac mini.spx.zip

To cut the chase: I use both HDD and SSD.

NoahAndrews commented 7 years ago

Are you running the firmware flasher from the HDD or the SSD? I see you still have the DMG file for the original flasher mounted. What happens if you run it from there, instead of copying it to your computer first?

chief-wrench commented 7 years ago

I have the system and all apps on SSD for fast starts, the /User folder is on the HDD.
Running the app from the DMG makes no difference.

chief-wrench commented 7 years ago

I might add theat there is currently no flashable controller connected to the mac. Maybe that makes the search for devices slow?

NoahAndrews commented 7 years ago

I've uploaded 3 different test versions here. Please give each one a try to see if it improves anything. Not all of them are actually functional. Only run interval.zip if one of the others fixes the issue.

Edit: exec.zip is broken. I'll reupload it in a minute.

NoahAndrews commented 7 years ago

exec-updated has been uploaded, and the original exec.zip has been renamed.

chief-wrench commented 7 years ago

I noticed that when the pointer hangs for the first time after starting the two hanging apps, the button "flash firmware" changes color from green to grey and stays like that. dunno if that helps any...

NoahAndrews commented 7 years ago

Ah good, I was hoping exec would work. It's funny, because the documentation says using exec should be less efficient than using execFile.

I noticed that when the pointer hangs for the first time after starting the two hanging apps, the button "flash firmware" changes color from green to grey and stays like that.

Thanks, that's a totally separate bug that I'll fix. The intended behavior is that it's gray when not ready and green when ready.

skullydazed commented 7 years ago

It's really weird that it hangs while executing dfu-programmer, no matter how that's executed. It should be executing in a separate thread from the UI so there shouldn't be any blocking going on. I haven't seen this happen on any of my macs either. Which release of macos are you running?

I'll do some experimenting to see if I can reproduce this on my machines. Maybe if I force a sleep() into dfu-programmer...

NoahAndrews commented 7 years ago

He's running 10.10.5 (Yosemite). I agree it shouldn't be happening, thanks for looking further into it. Is it OK if I go ahead and replace all execFile calls with exec for now at least?

skullydazed commented 7 years ago

Yeah, that seems ok to me. In fact I think if I had looked up the documentation first I would have used execFile(). I'm pretty sure that code was there when I started working with this.

NoahAndrews commented 7 years ago

execFile is what we were using. I'm switching to exec.

chief-wrench commented 7 years ago

hazy thought: I am aiming to flash Pro Micros. So to my knowlege the regular dfu-programmer for teensy at al is not installed on my machine. Maybe it tries to call a binary that SHOULD be bundled, but is not found either inside the package nor on my host machine? Maybe the call succeeds on your machines because you have it installed and so met some unknown requrements.

skullydazed commented 7 years ago

Sorry, 5:30am is probably not the best time to look at technical details. :) This is even stranger to me then.

chief-wrench commented 7 years ago

No need to hurry here. Get some rest.

skullydazed commented 7 years ago

@chief-wrench: All the binaries we call are bundled, and we specifically call the bundled version.

Your pro micro comment has me thinking though. Is that the only atmega32u4 device you have? You can't use dfu-programmer to flash a pro micro (or a teensy, actually) so it may be interfering with dfu-programmer somehow.

chief-wrench commented 7 years ago

The pro micro is not even connected to my machine.

skullydazed commented 7 years ago

That's actually good. It means we shouldn't need one to reproduce this.

NoahAndrews commented 7 years ago

Here's one way exec being faster than execFile could maybe make sense. If there's something about dfu-programmer that causes the process to take longer than it should to spawn, could that be reflected in the execFile call? Whereas exec is just spawning a shell, which would bear the slowdown from launching dfu-programmer.

skullydazed commented 7 years ago

That would only make sense if something about launching under the shell made it faster. While that's far from an impossible situation, I find it pretty unlikely. From a user's POV it shouldn't matter whether the delay is borne by the shell or dfu-programmer, it's the same delay either way.

NoahAndrews commented 7 years ago

My point was that using exec means we aren't directly spawning dfu-programmer. If the delay is in that step, then having the shell launch the process instead of our app would be a win.

At any rate, looking at the underlying nodejs code, I discovered that exec calls execFile, which is interesting:

function normalizeExecArgs(command /*, options, callback*/) {
  let options;
  let callback;

  if (typeof arguments[1] === 'function') {
    options = undefined;
    callback = arguments[1];
  } else {
    options = arguments[1];
    callback = arguments[2];
  }

  // Make a shallow copy so we don't clobber the user's options object.
  options = Object.assign({}, options);
  options.shell = typeof options.shell === 'string' ? options.shell : true;

  return {
    file: command,
    options: options,
    callback: callback
  };
}

exports.exec = function(command /*, options, callback*/) {
  var opts = normalizeExecArgs.apply(null, arguments);
  return exports.execFile(opts.file,
                          opts.options,
                          opts.callback);
};
NoahAndrews commented 7 years ago

@chief-wrench Please verify that this latest version of the app doesn't have the problem. If it does, it's because the change to exec may have broken starting dfu-programmer entirely in the version that fixed it before, and this version fixes that. So if this is broken, we're back to square 1. :/

https://drive.google.com/file/d/0B1MSp8fmGrzqcFVxVWtvdjRBQ2s/view?usp=sharing

chief-wrench commented 7 years ago

The issue is gone, you did it. Thanks a lot.

NoahAndrews commented 7 years ago

Great! I don't currently have privileges to close issues on this repository, so I'd appreciate it if you'd do so :)

chief-wrench commented 7 years ago

I very much like to do so, I just did not want to rush anything ;->

NoahAndrews commented 7 years ago

I think I found the underlying reason for the pausing problem. Because the packaged version on Mac uses an asar archive, and dfu-programmer is stored inside, the information in that link applies. Every time we used execFile to run dfu-programmer, it was extracting it to a temporary file.

@skullydazed We can either continue using exec, or we can tell electron-builder to unpack the dfu folder. I don't much care which.

NoahAndrews commented 7 years ago

Of course, the very next section of that document says to use ExecFile to execute programs from within an asar. Our best bet is probably to switch back, but to keep dfu-programmer unpacked.

NoahAndrews commented 7 years ago

I'll probably do that in the wix-installer branch, since that's where all of my packaging work is going.