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

bug fixed set sduino cc1101_rAmpl 42 #801

Merged
merged 2 commits into from
Feb 29, 2020
Merged

Conversation

elektron-bbs
Copy link
Contributor

  • Please check if the PR fulfills these requirements
  • Tests for the changes have been added / modified (needed for for bug fixes / features)
  • commandref has been added / updated (needed for bug fixes / features)
  • CHANGED has been updated (needed for bug fixes / features)
  • What kind of change does this PR introduce? (Bug fix, feature, docs update, ...)
    Bug fix

  • What is the current behavior? (You can also link to an open issue here)
    set sduinoIP cc1101_rAmpl 42
    2020.02.28 16:36:03 3: sduinoIP: setrAmpl, Setting AGCCTRL2 (1B) to -1 / 42 dB

  • What is the new behavior (if this is a feature change)?
    set sduinoIP cc1101_rAmpl 42
    2020.02.28 17:37:05 3: sduinoIP: setrAmpl, Setting AGCCTRL2 (1B) to 07 / 42 dB

  • Does this PR introduce a breaking change? (What changes might users need to make in their application due to this PR?)
    no

  • Other information:

@coveralls
Copy link

coveralls commented Feb 28, 2020

Pull Request Test Coverage Report for Build 2665

  • 4 of 4 (100.0%) changed or added relevant lines in 1 file are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.02%) to 53.486%

Totals Coverage Status
Change from base Build 2661: 0.02%
Covered Lines: 3230
Relevant Lines: 6039

💛 - Coveralls

@sidey79
Copy link
Contributor

sidey79 commented Feb 28, 2020

Verstehst Du, wieso set rAmpl 24 korrekt funktioniert`?

@elektron-bbs
Copy link
Contributor Author

elektron-bbs commented Feb 29, 2020

2020.02.29 11:23:37 3: sduinoIP: setrAmpl, v in for: 0
2020.02.29 11:23:37 3: sduinoIP: setrAmpl, v in for: 1
2020.02.29 11:23:37 3: sduinoIP: setrAmpl, v after for: 1
2020.02.29 11:23:37 3: sduinoIP: setrAmpl, v after sprintf: 00
2020.02.29 11:23:37 3: sduinoIP: setrAmpl, Setting AGCCTRL2 (1B) to 00 / 24 dB

Ich verstand eher nicht, das "set rAmpl 42" funktioniert.
$v wird erst erhöht und dann die Prüfung "$v < @ampllist" durchgeführt. Da wir vorher $v definieren, bleibt $v nach der for-Schleife erhalten:

2020.02.29 11:24:20 3: sduinoIP: setrAmpl, v in for: 0
2020.02.29 11:24:20 3: sduinoIP: setrAmpl, v in for: 1
2020.02.29 11:24:20 3: sduinoIP: setrAmpl, v in for: 2
2020.02.29 11:24:20 3: sduinoIP: setrAmpl, v in for: 3
2020.02.29 11:24:20 3: sduinoIP: setrAmpl, v in for: 4
2020.02.29 11:24:20 3: sduinoIP: setrAmpl, v in for: 5
2020.02.29 11:24:20 3: sduinoIP: setrAmpl, v in for: 6
2020.02.29 11:24:20 3: sduinoIP: setrAmpl, v in for: 7
2020.02.29 11:24:20 3: sduinoIP: setrAmpl, v after for: 8
2020.02.29 11:24:20 3: sduinoIP: setrAmpl, v after sprintf: 07
2020.02.29 11:24:20 3: sduinoIP: setrAmpl, Setting AGCCTRL2 (1B) to 07 / 42 dB

@sidey79
Copy link
Contributor

sidey79 commented Feb 29, 2020

Meinst Du damit, dass die Register auch schon bei der Grep Variante richtig gesetzt wurden oder wie soll ich deine Frage verstehen?

Mich würde halt interessieren, wieso der Test mit dem Wert 24 fehlerfrei auch vor dem PR funktionierte, es aber bei 42 nicht klappt oder liege ich falsch?

@elektron-bbs
Copy link
Contributor Author

elektron-bbs commented Feb 29, 2020

Ich glaube, wir verstehen uns jetzt falsch. Ich habe keine Frage gestellt, sondern versucht, deine Frage zu beantworten.
Ob es überhapt einen Test gibt, weiß ich nicht.
Bei der Variante mit grep funktionierten alle Werte von 24 bis 40, nur eben 42 nicht, weil es in der Liste keinen Wert größer 42 gibt.

grep { $ampllist[$_] > $a[1] }

@sidey79
Copy link
Contributor

sidey79 commented Feb 29, 2020

Ich verstand eher nicht, das "set rAmpl 42" funktioniert.

Dieser Satz hatte mich irritiert. Jetzt ist das ja aber geklärt, dass alles bis auf 42 funktionierte.
Einen Test gibt es nur mit 24 und dadurch ist es nicht aufgefallen. Ich schätze mir ist auch klar, wieso das mit 42 nicht geklappt hat.

@elektron-bbs elektron-bbs merged commit b368f40 into dev-r34 Feb 29, 2020
@elektron-bbs elektron-bbs deleted the dev-r34_set-cc1101_rAmpl branch March 12, 2020 20:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants