Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Small changes (signal strength text + get NRF signal strength). #1119

Merged
merged 3 commits into from
Aug 27, 2023
Merged

Small changes (signal strength text + get NRF signal strength). #1119

merged 3 commits into from
Aug 27, 2023

Conversation

DanielR92
Copy link
Collaborator

Hello everyone,

I've added a little improvement here for you.

Among other things, the labeling of the selected signal strength from the NRF. I added "recommended" here. So everyone knows that this is the first choice and then has no questions about the strength.

Original wish: #1050
The bigger change is the feature request requested by Knickohr with signal recognition for the NRF. - Important: Someone has to check this in advance, I seem to have a bad signal all the time (in my case, the connection only builds up after a long wait).

So please check here!

@DanielR92 DanielR92 added the question Further information is requested label Aug 24, 2023
@DanielR92
Copy link
Collaborator Author

grafik

@DanielR92 DanielR92 marked this pull request as ready for review August 24, 2023 12:16
@DanielR92
Copy link
Collaborator Author

DanielR92 commented Aug 24, 2023

Info: die Infos werden noch nicht über MQTT freigegeben.
Das muss ich mir anschauen, bin mir unsicher hier wie es am besten umgesetzt werden soll.

Ich melde mich wenn ich was passendes dazu gecoded habe.

So sieht es in der API aus:
grafik

@knickohr
Copy link

Ich würde es so wie in der API machen good_signal : true/false

bool goodSignal = mNrf24.testRPD();
DPRINT(DBG_INFO, F("NRF Signal: "));
DPRINT(DBG_INFO, String(goodSignal));
mNrf24.read(0,0);
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

was bedeutet diese Zeile?

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Die Zeile gibt offenbar ein good/bad (true/false) aus, das die Signalstärke (< oder > -64dBm) der einzelnen Inverter darstellt. Siehe Issue #1050

Copy link
Owner

@lumapu lumapu Aug 26, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

das ist doch schon in Zeile 244. Mir geht es um Zeile 247

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Axo 😇

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Die Funktion wird aufgerufen sobald "Settings" aufgerufen wird.
Das heißt es wird nicht jedesmal im Code abgefragt wie gut das Signal ist.

"Macht man ja nur einmalig, damit man einen groben Wert hat."

245 + 246 -> Hupps, die können raus, war für mich zum debuggen noch. Jedoch hab ich mich wie ein Schnitzel gefreut das es direkt auf der Settings-Seite auch angezeigt wurde, das ich das so gesaved habe. :D Kann man raus nehmen, sollte man rausnehmen.

src/hm/hmRadio.h Outdated
@@ -21,7 +21,7 @@
#define ALL_FRAMES 0x80
#define SINGLE_FRAME 0x81

const char* const rf24AmpPowerNames[] = {"MIN", "LOW", "HIGH", "MAX"};
const char* const rf24AmpPowerNames[] = {"MIN (recommended)", "LOW", "HIGH", "MAX (experimental)"};
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

das brauchen wir mMn. nicht, ist nur intern

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Achso! Sorry, wusste nicht ganz genau wie das weiter zur Seite geschliffen wird. Wenn das nur intern verwendet wird, wieso nutzen wir hier kein uint, statt chars? Können hier platz sparen.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

da das auf der Seriellen Konsole angezeigt wird, da will man doch sofort wissen welches Level und nicht als Zahl 😉

@@ -50,7 +50,7 @@
}

function parseRadio(obj, stat) {
const pa = ["MIN", "LOW", "HIGH", "MAX"];
const pa = ["MIN (recommended)", "LOW", "HIGH", "MAX"];
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hier würde ich "recomemended" auch weglassen

Copy link
Collaborator Author

@DanielR92 DanielR92 Aug 26, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, ich sehe es anders. Hier soll jeder sehen das es mit MIN zu empfehlen ist.
Erst dann langsam "hochschrauben".

@lumapu
Copy link
Owner

lumapu commented Aug 27, 2023

@DanielR92 danke für die Änderungen, PR fließt in 0.7.42 ein 😀

lumapu added a commit that referenced this pull request Aug 27, 2023
* add signal strength for NRF24 - PR #1119
@lumapu lumapu merged commit eee64e7 into lumapu:development03 Aug 27, 2023
@knickohr knickohr mentioned this pull request Aug 28, 2023
16 tasks
@DanielR92 DanielR92 deleted the small-changes branch September 6, 2023 17:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
question Further information is requested
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants