Couple of bugs found during code review

[Emile666]

While reviewing the code, I found a number of (potential) bugs:

1. L118 & L119: the cfgbits field is not filled in for GAL26CV12 and GAL26V12. I don't know why I don't get a warning from the compiler, strange. But there really is one data byte missing.
2. L638: if (verbose & bigRam), this should be a double & (verbose && bigRam). A single & is a bitwise operator
3. L1151 L1221: the function sendLine expects an integer for the 2nd parameter (bufSize), but it is given the value of GALBUFSIZE, which is 256 * 1024 or an uint32_t. It now overflows and probably passes a value of 0.

Please advice how to solve these.

[Emile666]

If the config bits are the S0, S1 etcetera bits in the macro-cells, then the values for the cfg-bits for GAL26CV12 and GAL26V12 are 24 (S0,S1: 2x12 macro-cells) and 48 (S0..S3: 4x12 macro-cells) respectively. Please confirm if this is correct.

I did set GALBUFSIZE to (31 * 1024) just to get rid of this bug. But don't know how much it should be. If it needs to be 256 * 1024, then the function itself has to change, so that bufSize can be an uint32_t.

[ole00]

Thanks for the review. The issue 1: compiler automatically fills-in the rest of array by 0. Fortunately the config bit value is not used in the PC App, but I've added the correct values (as you found out) 24 and 48 just for completeness. Issue 2: yes, logical operator is better, but even with bitwise operator the condition should work OK. Issue 3: my line 1152 in afterburner.c shows the following code:

    //set new PES
    sprintf(buf, "#p %s\r", pesString);
    sendLine(buf, MAX_LINE, 300);

and MAX_LINE is currently defined as (16*1024), so that should not be an issue.

[Emile666]

Thanks very much for answering! I am impressed by the overall hardware and software design and by how well it works. I do have some issues with the software, but only wish to improve an already excellent piece of work!

When reporting issue 3, I clearly made a mistake by reporting the wrong line-numbers, my bad! The define GALBUFSIZE is defined at line 62 as (256 * 1024), which is a long integer, and is used at line 1221: "readSize = sendLine(buf, GALBUFSIZE, 12000);"
Since the 2nd parameter of sendLine is an integer, this will create an overflow.

I will update my post above to correct the linenumbers.

[Emile666]

Another issue I came across, is the VPP measurement. I added a detailed analysis to issue #54 already reported by HubertusHirsch. I largely agree with him and found the same (inaccurate) measurements. I also added some code-improvements on how to resolve this. Readings are now accurate within 0.05 Volts and can be made even smaller without adding offsets.

Will you please have a look at my comments and proposal in issue #54? I am happy to help with a pull-request or sending in source files if you wish.