arduino / arduino/ArduinoCore-API

Dubious code in printFloat(double number, uint8_t digits)

オープン
#172 コメント 2 件 リアクション 0 件 担当者 0 名 GitHub で見る
bug
主要言語
C++
スター
306
フォーク
150
PR マージ指標
30日以内にマージされた PR はありません

説明

A function named printFloat that prints a double is unfortunate naming!

https://github.com/arduino/ArduinoCore-avr/blob/2ff00ae7d4e85fa422b7918ee12baf56a1f3006e/cores/arduino/Print.cpp#L229

The comment indicates a problem. It works but empirical constants are not good practice!

The range check is needed in order to prevent the integer part of the float overflowing in
https://github.com/arduino/ArduinoCore-avr/blob/2ff00ae7d4e85fa422b7918ee12baf56a1f3006e/cores/arduino/Print.cpp#L247
unsigned long is not necessarily 32 bits so the numeric literal is non-portable.
The constant is actually the unsigned long equivalent of INT_MAX - 1. (The -1 is to allow for rounding but further thought needed to be certain of that).

I have a fix which avoids the need for int_part so the problem no longer arises. It also makes the implementation of %g and %e formats relatively trivial.

* Some testing but far from thorough.
* Uses sig figs for %f format instead of decimal places but that can be changed.
* Precision as a parameter not implemented.
* Consideration should be given to ```printFloat(float f, struct PrintOptions *options)```
* It prints a float but that is more than adequate for most purposes.

I would welcome feedback on the various options before I do anything further.

```
#define F_LARGE 6 /* maximum exponent for F format */
#define F_SMALL -3 /* minimum exponent for F format */
#define E_SIG_FIG 4 /* significant figures for E format */
#define F_SIG_FIG 6

// Buffer size. The +1 allows for the rounding digit.
#if E_SIG_FIG > F_SIG_FIG
#define SIZE (E_SIG_FIG + 1)
#else
#define SIZE (F_SIG_FIG + 1)
#endif

void printFloat(float f) {
int8_t d;
char buffer[SIZE];
bool negative;
bool Eformat = false;
uint8_t start = 1; // index of first digit leaving room for a carry from the rounding
uint8_t dp; // index of first digit after the decimal point
uint8_t finish; // index of extra digit used for rounding
int8_t i; // loop counter (must be signed)

if (isnan(f)) { print("nan"); return; }
if (isinf(f)) { print("inf"); return; }

negative = f < 0.0;
if (negative) { f = -f; print('-'); }

int8_t exponent = 0;
while (f >= 10.0) { exponent++; f /= 10.0; }
while (f < 1.0) { exponent--; f *= 10.0; }

// need one more digit for use when rounding
if (exponent > F_LARGE || exponent < F_SMALL) {
// E format
finish = start + E_SIG_FIG;
dp = 1;
Eformat = true;
}
else {
// F format
finish = start + F_SIG_FIG;
dp = exponent + 1;
}

// store the digit chars into the buffer
for (uint8_t i = start; i <= finish; i++) {
d = (uint8_t)f;
buffer[i] = '0' + d;
f -= d;
// Could check for f==0 here and save some multiplies but larger code size
f *= 10.;
}

// rounding
if (buffer[finish] >= '5') {
i = finish - 1;
buffer[0] = '0';
while (buffer[i] == '9') {
buffer[i] = '0';
i--;
}
buffer[i]++;
}

if (buffer[0] == '1') {
start = 0; // there was a carry from the rounding
}
else {
dp++;
}

if (exponent >= 0) { // positive exponent
for ( i = start; i < dp; i++) {
print(buffer[i]);
}
print('.');
for ( i = dp; i < finish; i++) {
print(buffer[i]);
}
}
else { // negative exponent
if (Eformat) {
print(buffer[start]);
print('.');
for ( i = dp; i < finish; i++) {
print(buffer[i]);
}
}
else { // F format
print("0.");
for (i = 1; i < -exponent; i++) {
print('0');
}
for (i = start; i < finish; i++) {
print(buffer[i]);
}
}
}

if (Eformat) {
print('E');
print(exponent);
}
}
```

コントリビューションガイド

このリポジトリのコントリビューションガイドは索引されていません

調査の方向性

まず cores/arduino/Print.cpp、特に printFloat と関連する範囲チェックコードを確認してください。実装を変更する前に、望ましいフォーマット、精度パラメーター、float と double の動作の違い、および %g と %e を対象に含めるかどうかを明確にしてください。完了とするには、これらの選択肢について合意し、記載されている限定的なテストを超えるテストを行う必要があります。

索引モデルが issue の本文から書いたものです。

評価

技術スタック
cpp
領域
embedded-iot
issue の種類
リファクタリング
難易度
5/5
見積もり時間
1週間以上
活発さ
停滞
明瞭さ
説明が足りない
初心者へのやさしさ
25/100

新しい issue をメールで受け取る

初心者向けの GitHub issue を短くまとめたダイジェスト。