gusmanb / micropicodrive

Internal microdrive replacement for the Sinclair QL
MIT License
13 stars 1 forks source link

Not an issue by now but function capped out #9

Closed Silveriomrs closed 2 months ago

Silveriomrs commented 2 months ago

Good afternoon,

Sorry for bothering you again. Checking some part of the code, in this case managing the DMA control. I've found an conflict of functions ( I have not enough level to evaluate the consequences yet).

Status values always true

How the conditions are enums, and the comparing between the 2 terms are separated by ORs, it will be always True. The Ifs also are linked so the if-else will never happen. In the case it happens... will be always true too due to the same reason, the OR makes it always bigger or smaller than the reference.

Sorry if I don't explain myself properly.

Thank you.

gusmanb commented 2 months ago

Yes, it's a typo. it should be like this:

    if(activeStatus >= MDA_READ_HEADER_GAP && activeStatus <= MDA_READ_SECTOR)
        disable_DMAs(true);
    else if(activeStatus >= MDA_WRITE_HEADER_GAP && activeStatus <= MDA_WRITE_SECTOR)
        disable_DMAs(false);

Can you update it in your branch and check if it works properly? if it works I will update the source, right now I'm really busy with my work and don't have time to do a proper testing.

In theory the error will not affect the device as it only stops the DMA's while they aren't used, it is mostly for sanity.

Silveriomrs commented 2 months ago

I'll do it tonight. Just I wrote it here cause I can't determinate side effects farther than to check if it continues working as "usual" or not. ASAP I'm going to tell you how it goes.

But till tomorrow I won't be able to try it out connected to a QL, only the browsing powered it by USB.

See you later then with news.

Silveriomrs commented 2 months ago

Good evening, It seems to work fine while I browse the SD and load something. Saving feature looks also working but I cannot check it out till tomorrow or later.

In theory the error will not affect the device as it only stops the DMA's while they aren't used, it is mostly for sanity.

About it, considering the code order, once that function was called, the DMA remained disabled till a new reset or power up the device again. In other words... DMA was not used at all after that function.

Tomorrow I'm going to do more tests connected to my QL.

Keep in mind that I have modified a part of the original code. Not that part (MicroDriveControl), but always there is the possibility that could/couldn't work for me and for yours be fine/wrong.

Could you tell me when that function is called? I mean deselect_md(), that way I could try to force the test oriented to detect problems with it.

Tomorrow more news.

gusmanb commented 2 months ago

Good evening, It seems to work fine while I browse the SD and load something. Saving feature looks also working but I cannot check it out till tomorrow or later.

In theory the error will not affect the device as it only stops the DMA's while they aren't used, it is mostly for sanity.

About it, considering the code order, once that function was called, the DMA remained disabled till a new reset or power up the device again. In other words... DMA was not used at all after that function.

Tomorrow I'm going to do more tests connected to my QL.

Keep in mind that I have modified a part of the original code. Not that part (MicroDriveControl), but always there is the possibility that could/couldn't work for me and for yours be fine/wrong.

Could you tell me when that function is called? I mean deselect_md(), that way I could try to force the test oriented to detect problems with it.

Tomorrow more news.

Well, you are totally wrong :)

If DMA's were not used, the device would not work at all, it would be a very nice brick. DMAs are automatically rearmed whenever there is any operation and disarmed when a transfer is finalized, so as I stated that code is there only for sanity, is not critical at all.

Do not consider "code order", that's nonsensic, you need to follow the code execution paths, and in this case is very easy as the implementation is basically a bunch of state machines.

The function is executed when... the microdrive is deselected :D Read from MD1, read from MD2 and go back and forth, each time you change from one drive to the other the one what was active is deselected.

gusmanb commented 2 months ago

I'm going to close the issue as the patch works correctly.