views:

676

answers:

2

SOLVED

I am using delphi 2009. My program listens for usb drives being connected and remove. Ive used a very similar code in 10 apps over the past year. It has always worked perfectly. When i migrated i had to give up using thddinfo to get the drive model. This has been replaced by using WMI. The WMI query requires the physical disk number and i happen to already have a function in the app for doing just that.

As i test I put this in a button and ran it and it successfully determines the psp is physical drive 4 and returns the model (all checked in the debugger and in another example using show message):

function IsPSP(Drive: String):Boolean;
var
Model: String;
DriveNum: Byte;
begin
  Result := False;
  Delete(Drive, 2, MaxInt);
  DriveNum := GetPhysicalDiskNumber(Drive[1]);
  Model := (MagWmiGetDiskModel(DriveNum));
  if Pos('PSP',Model) > 0 then Result := True;
end;

procedure TfrmMain.Button1Click(Sender: TObject);
var DriveNum: Byte;
begin
  IsPSP('I');
end;

It works perfectly that is until i allow the WMDeviceChange that ive been using for a year to call up the getphysicaldisknumber and the wmi query statement. Ive tried them by themselves theyre both a problem. GetPhysicalDiskNumber freezes real bad when its doing a CloseHandle on the logical disk but does return the number eventually. The WMI query fails with no error just returns '' debugger points into the wbemscripting_tlb where the connection just never happened. Keep in mind the only thing thats changed in a year is what im calling to get the model i was using an api call and now im using something else.

Below is the rest of the code involved at this time sans the ispsp that is displayed above:

procedure TfrmMain.WMDeviceChange(var Msg: TMessage);
var Drive: String;
begin
  case Msg.wParam of
    DBT_DeviceArrival: if PDevBroadcastHdr(Msg.lParam)^.dbcd_devicetype = DBT_DevTyp_Volume then
      begin
        Drive := GetDrive(PDevBroadcastVolume(Msg.lParam)) + '\';
        OnDeviceInsert(Drive);
      end;
    DBT_DeviceRemoveComplete: if PDevBroadcastHdr(Msg.lParam)^.dbcd_devicetype = DBT_DevTyp_Volume then
      begin
        Drive := GetDrive(PDevBroadcastVolume(Msg.lParam)) + '\';
        OnDeviceRemove(Drive);
      end;
  end;
end;

Procedure TfrmMain.OnDeviceInsert(Drive: String);
var PreviousIndex: Integer;
begin
  if (getdrivetype(Pchar(Drive))=DRIVE_REMOVABLE) then
  begin
    PreviousIndex := cbxDriveList.Items.IndexOf(cbxDriveList.Text);
    cbxDriveList.Items.Append(Drive);
    if PreviousIndex = -1 then //If there was no drive to begin with then set index to 0
    begin
      PreviousIndex := 0;
      cbxDriveList.ItemIndex := 0;
    end;
    if isPSP(Drive) then
    begin
      if MessageDlg('A PSP was detect @ ' + Drive + #10#13 + 'Would you like to select this drive?',mtWarning,[mbYes,mbNo], 0) = mrYes then
      cbxDriveList.ItemIndex := cbxDriveList.Items.IndexOf(Drive)
      else cbxDriveList.ItemIndex := PreviousIndex;
    end
    else if MessageDlg('USB Drive ' + Drive + ' Detected' + #10#13 + 'Is this your target drive?',mtWarning,[mbYes,mbNo], 0) = mrYes then
        cbxDriveList.ItemIndex := cbxDriveList.Items.IndexOf(Drive)
    else cbxDriveList.ItemIndex := PreviousIndex;
  end;
end;

Procedure TfrmMain.OnDeviceRemove(Drive: String);
begin
  if not (getdrivetype(Pchar(Drive)) = DRIVE_CDROM) then
  begin
    if cbxDriveList.Text = (Drive) then ShowMessage('The selected drive (' + Drive + ') has been removed');
    cbxDriveList.Items.Delete(cbxDriveList.Items.IndexOf(Drive));
    if cbxDriveList.Text = '' then cbxDriveList.ItemIndex := 0;
    if Drive = PSPDrive then //Check Detect PSP and remove reference if its been removed
    begin
      PSPDrive := '';
    end;
  end;
end;

Rob has said something below about im not calling the inherited message handler, ive read the document i see a couple of things i can return... but im not really sure i understand but i will look into it. Im not a very good pascal programmer but ive been learning alot. The transition to 2009 has had some rough patches as well.

The USB drive detection and all that works perfectly. If i remove the two things from is psp the user is greeted right away with wis this your whatever and adds I:\ to the list. Its just the two new things that have changed in the app that fail when called by wmdevicechange and as said before they work on their own.

EDIT - SOLVED

Alright well im using a timer as suggested and the problem seems to be solved. One note is that when called by the timer very shortly after the wmdevicechange getting the physical disk number still seems to be slow. I attribute this to the device still being attached to the system.

On that note im using a P2 450 on the regular. I hooked the PSP and app to a 1.8Ghz Dual Core Laptop and the program detected the psp and notified the user very fast. So the app wont freeze unless there on a very very slow computer and on this slow onw its only for a matter of seconds and doesnt affect the operation of the program though isnt very cool. But i feel that all modern computers will run the detection fast especially because they can attach the device alot faster.

+2  A: 

You haven't indicated what "statement 1" is in your code.

I have a few comments about parts of the code, which may or may not be related to the problem you're having.

First, you assign a value to DriveNum in IsPSP, but you don't use it. The compiler should have issued a hint about that; don't ignore hints and warnings. You also pass the magic number 4 into MagWmiGetDiskModel; was that supposed to be DriveNum instead?

You aren't calling the inherited message handler, and you aren't returning a result in your message handler. The documentation tells what values you're supposed to return. To return a value from a Delphi message handler, assign a value to the Msg.Result field. For the cases that your message handler doesn't handle, make sure you call inherited so that the next handler up the chain can take care of them. If there is no next handler, then Delphi will call DefWindowProc to get the operating system's default behavior.

The change you've illustrated is called refactoring, and it will do nothing to affect how your code runs. It makes the code easier to read, though, so please keep the second version. As for finding the problem, my best advice is to use the debugger to step through the code to identify the point where things stat to go wrong and the parts that run slower than you'd like. You can also try removing portions of the code to confirm that the other parts work correctly in isolation.

Rob Kennedy
Thanks rob, the code isnt 100% correct because ive been trouble shooting 4 is the psp at this time but drivenum would normally be set by the now discluded getphysicaldisknumber statement.
Brian Holloway
The WMDeviceCHange works perfectly has for a year but now that im not useing an api call to get the disk model and instead am using 2 new things all of a sudden there is a problem. The returned value is turned into a drive letter by another function which is exactly what i want.
Brian Holloway
I know exactly where it goes wrong and exactly where it slows down. I removed them and ran it and its perfect. This is why the new questione xists because i did not figure it out on the previous. I also stated the troubled code works fine by itself.
Brian Holloway
+2  A: 

It's possible that the information you're querying becomes available only after the WMDeviceChange message handler runs. If the very same code works when called from a button, try this:

  1. Refactor your WMDeviceChange handler code into one or more separate methods.
  2. In the WMDeviceChange handler, activate a precreated timer and have it fire one second later, or something like that.
  3. Call the former WMDeviceChange handler code from the timer handler code.
Mihai Limbășan
Instead of a timer, you can post a message back to the form. Then you don't have to worry about disabling the timer afterward, and it doesn't look like it's a periodic event.
Rob Kennedy
I think you might be right. Brian asked the same question before, but with better information about what's really wrong. http://stackoverflow.com/questions/645338/delphi-magsys-wmi-more-works-in-procedure-but-not-function-function-returns
Rob Kennedy
I reread this twice and while initially I see you have the same thought i did, but you say refactor the code well i did as best as i can and as rob says wont change anything. The timer goes along my thought that i need something else to fire the event. I will look into it, thank you.
Brian Holloway
Sorry, didn't mean to imply refactoring your code would make anything work as such - I just meant it as a precautionary step against copypasting code into a handler for a different message, and against having a lot of code directly in a handler method, which is error-prone.
Mihai Limbășan
One issue im having in my mind is what todo with the drive letter. Im afraid that lets say someone plugs in their usb hub and all of a sudden there is a bunch of letters to deal with. My first thought is to use an instance of timer but i guess for now ill write a timer that has a variable.
Brian Holloway
Ive done the timer and all i can say is WIN, now if u could tell me that the WMDeviceChange wont outrun a timer set to an interval of 1 then i wouldnt be so scared of 2+ drives being connected at the same time.
Brian Holloway
Of course there's a race condition there, and you can't guarantee who will win. However, you *can* detect when the information gathering fails. In that case, you've called the timer too soon. Have it re-enable itself after one more second, and so on :) That's basically an automatic retry.
Mihai Limbășan