1
\$\begingroup\$

Apple's System Management Controller uses FPE2 and SP78 as floating-point formats.

While writing a Java library that that interfaces with the SMC, I ported someone's C function to convert read a FPE2-formatted byte array as a float.

public static float strtof(byte[] bytes, int size, int e) {
    int total = 0;
    // Add up bits to left of fractional bits
    for (int i = 0; i < size; i++) {
        if (i == (size - 1))
            total += (bytes[i] & 0xff) >> e;
        else
            total += (bytes[i] & 0xff) << (size - 1 - i) * (8 - e);
    }
    // Mask fractional bits and divide
    return total + (bytes[size - 1] & (1 << e) - 1) / (float) (1 << e);
}

The bytes and size parameters come from reading a structure in Apple's SMC interface. The array is a byte[32] value in a structure, where a second part of the structure states how many of the elements of the array are significant. In the use case, getting fan speed, bytes[0] and bytes[1] will be populated with data (the other 30 bytes are ignored) and size will have a value of 2.

In the given code, the value of e represents how many rightmost bits out of the size bytes are for the fractional part. If e is 2 (such as for FPE2), then the final 2 bits can have values 0, 1, 2, or 3 representing the fractions 0.0, 0.25, 0.5, and 0.75. The remaining leading bits are interpreted as the integer part.

Example input:

  • bytes = { 0x00, 0x05, 0x00, 0x00, ... } // First 2 bytes binary 101
  • size = 2
  • e = 2

Example output: 1.25 // Integer portion is 1, fraction is 1/4.


Did I do it right (assuming arguments are already validated: e will never be more than 8, and size will never be more than 4)? Points of concern are order of operation, what happens bitwise when I promote a byte to an int and then shift it.

While this function converts an FPE2 format number, I'm trying to make it a bit more general. A similar format, SP78, is used for temperature. The rightmost 8 bits (divided by 256) are the fractional portion, the left 8 bits are a signed byte. (This method is not intended to handle the signed calculation but this is given as a reference.)

\$\endgroup\$
2
\$\begingroup\$
public static float strtof(byte[] bytes, int size, int e) {

Could use some documentation explaining what size and e are. I think size is the number of bytes to process, and e is the number of bits corresponding to the fractional part.


    int total = 0;
    // Add up bits to left of fractional bits
    for (int i = 0; i < size; i++) {
        if (i == (size - 1))
            total += (bytes[i] & 0xff) >> e;
        else
            total += (bytes[i] & 0xff) << (size - 1 - i) * (8 - e);
    }
    // Mask fractional bits and divide
    return total + (bytes[size - 1] & (1 << e) - 1) / (float) (1 << e);

I see a couple of apparent bugs here:

  1. I understood that the output should be unsigned. But then you need to use long rather than int.
  2. total += (bytes[i] & 0xff) << (size - 1 - i) * (8 - e); means that if e == 8 then all of the bytes are added without shifting. Ugh.

But frankly the bit-bashing seems overcomplicated. Unless I'm missing something, you could simplify to

    long total = 0;
    for (int i = 0; i < size; i++) {
        total = (total << 8) + (bytes[i] & 0xff);
    }
    return total / (float) (1 << e);
\$\endgroup\$
  • \$\begingroup\$ I do have javadoc that I didn't copy here... sorry. Understand and had considered bug 1... and Wow, good catch on bug 2. Didn't see that! And in my quest to faithfully reproduce the original code, I totally missed your simplification. Thanks! \$\endgroup\$ – Daniel Widdis Apr 27 '16 at 16:42
  • 1
    \$\begingroup\$ A tiny nitpick: when doing bitwise operations like << and &, I would prefer to see the bitwise operator | instead of the arithmetic operator +. \$\endgroup\$ – 200_success Apr 27 '16 at 21:20
  • \$\begingroup\$ Another error that I found later was that in the given code I should have used the unsigned right shift operator >>> rather than >>. However, thanks to the simplification in this answer, that error disappeared. I ended up implementing this code in two steps, with the final float division in the method I desired, and the remainder in a separate method converting to a long (which I needed for other code). \$\endgroup\$ – Daniel Widdis Apr 29 '16 at 23:52
2
\$\begingroup\$

Documentation

I took me a while to understand and research the algorithm and the purpose of the function parameters. It would be better to write some Javadoc for reviewers and future readers (which may include yourself).

Identifiers

You should use variable and function identifiers that explain their purpose (barring obvious cases like i for a loop iterator).

  • strtof is a function defined since C89 and does something very different from your function; that's not a good name here. What about FPxToFloat because the function converts data in a variant of the FP* formats (x instead of *) to a float?

  • e is the number of bits in the integral part of the number. How about integerBits instead?

  • total – My first thought was “total of what?”. I usually go for a name like result in these cases since it holds the function result (or an intermediate result on the way to the final result) which should be well documented somewhere nearby.

Limitations

  • Why is size restricted to ≤ 4 bytes and integerBits restricted to ≤ 8. You say you want a more generic solution. I don't see a reason why size can't be 17 and integerBits 49.

  • Currently the encoded number needs to start at offset 0 inside the bytes array. Since Java doesn't have C's pointer arithmetic it would be useful to offer an offset parameter like the read and write functions in java.io.

    You can still offer a convenience function with less boiler plate:

    public static float FPxToFloat(byte[] bytes, int integerBits) {
        return FPxToFloat(bytes, 0, bytes.length, integerBits);
    }
    

Complexity

Your implementation seems overly complicated and suffers from a number of unnecessary limitations (see first point in the section above). What about this:

double result = 0;
int exponent = integerBits - Byte.SIZE;
for (; offset < size; offset++, exponent -= Byte.SIZE) {
    result += Math.scalb(bytes[offset] & 0xFF, exponent);
}
return result;

scalb avoids multiplicative floating-point operations and overflow of integers that shall later be converted to floating-point. The only optimization I can think of is to work on long instead of byte if possible to use the available processor word size better (to achieve that one can wrap byte[] inside a java.nio.ByteBuffer and use a LongBuffer view onto it).

Full program (Ideone is currently acting up here so a Gist it has to be for now.)

\$\endgroup\$
  • \$\begingroup\$ Thanks for the feedback. I understand your comments about identifiers and will update them. For reference, I was porting the code from C here: github.com/Chris911/iStats/blob/master/ext/osx_stats/smc.c \$\endgroup\$ – Daniel Widdis Apr 27 '16 at 16:00
  • \$\begingroup\$ I like the name FPxx to Float. Note the E is hex for 14 bits, so you could have FPC4, FPF1, FP88, etc. Another format I am parsing is SP78, similar except the "S" means it's signed. Since e will never be > 8, there is no need to handle 2^e > Integer.MAX_VALUE. \$\endgroup\$ – Daniel Widdis Apr 27 '16 at 16:10
  • \$\begingroup\$ Thanks for the clarification. I included that in my edit. \$\endgroup\$ – David Foerster Apr 27 '16 at 18:58
1
\$\begingroup\$

The function signature is unnatural to Java.

The xtoy naming scheme is unusual, especially considering that Java, unlike C, knows the type of x — and in any case, byte[] is not the same as String!

Specifying the size is redundant, and therefore probably harmful, since it introduces an unnecessary opportunity for inconsistency. (Allowing both the offset and length to be specified might be useful, but that's not what you're doing here.)

@PeterTaylor's solution looks right to me. It could be simplified even further by taking advantage of the BigInteger(byte[]) constructor:

public static float fromFPE2(byte[] bytes, int e) {
    return (float)(new BigInteger(bytes).doubleValue() / (1 << e));
}
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.