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.)
3 Answers 3
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:
- I understood that the output should be unsigned. But then you need to use
long
rather thanint
. total += (bytes[i] & 0xff) << (size - 1 - i) * (8 - e);
means that ife == 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);
-
\$\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– Daniel Widdis2016年04月27日 16:42:42 +00:00Commented Apr 27, 2016 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– 200_success2016年04月27日 21:20:17 +00:00Commented Apr 27, 2016 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– Daniel Widdis2016年04月29日 23:52:20 +00:00Commented Apr 29, 2016 at 23:52
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 aboutFPxToFloat
because the function converts data in a variant of theFP*
formats (x
instead of*
) to afloat
?e
is the number of bits in the integral part of the number. How aboutintegerBits
instead?total
– My first thought was "total of what?". I usually go for a name likeresult
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 andintegerBits
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 anoffset
parameter like theread
andwrite
functions injava.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.)
-
\$\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– Daniel Widdis2016年04月27日 16:00:04 +00:00Commented Apr 27, 2016 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– Daniel Widdis2016年04月27日 16:10:54 +00:00Commented Apr 27, 2016 at 16:10 -
\$\begingroup\$ Thanks for the clarification. I included that in my edit. \$\endgroup\$David Foerster– David Foerster2016年04月27日 18:58:32 +00:00Commented Apr 27, 2016 at 18:58
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));
}
Explore related questions
See similar questions with these tags.