Ds18B20 Zeigt zu höhe Werte

[Dennis T. (dennist92)]

Ich habe einen mega328p mit einem DS18B20 Programmiert, die 
Temperaturausgabe geschieht über ein LCD Display. Anfangs habe ich es 
nicht hinbekommen, dass die Temperatur ausgegeben wird, der Sensor hat 
aber reale Werte geliefert, ca. 0x16B. Nun habe ich es hinbekommen, dass 
die Temperatur ausgegeben wird, aber nun gibt der Sensor mir einen Welt 
von 0x6D2 also über 100 Grad. Wo liegt der Fehler?
DS.c Datei

int ds_readtemp(){  
  ds_masterreset();
  ds_command(DS_SKIPROMCOMMAND);
  ds_command(DS_CONVERTTEMP);
  _delay_ms(DS_WAIT_MS);
  ds_masterreset();
  ds_command(DS_SKIPROMCOMMAND);
  ds_command(DS_READSCRATCH);
  uint8_t temp1 = ds_readbyte();
  uint8_t temp2 = ds_readbyte();
  int out = ((temp2) * 256) + temp1;
  return out;
}
int ds_readbit(){
  ds_out();
  DS_PORT &= ~(1 << DS_PIN);
  ds_in();
  _delay_us(1);
  if (PINC & (1 << DS_PIN)){
    return 1;
  }
  return 0;
} 
int ds_readbyte(){
  int i;
  int i_byte = 0;
  for (i = 0; i < 8; i++){
    i_byte += (ds_readbit() << i);
  }
  return i_byte;
}

hier habe ich eigentlich nur die letzten 2 Zeilen in ds_readtemp 
geändert, diese haben aber mit dem auslesen an sich eigentlich nichts 
zutun.

[(prx) A. K. (prx)]

Das Timing stimmt nicht.

[(prx) A. K. (prx)]

Ist hier wohl kein Problem, aber generell zum Umgang mit dem I/O-Pin bei 
separat versorgtem Sensor, also nicht parasitär über die Datenleitung: 
Bit im Output-Register einmalig auf 0 setzten und dann dabei belassen. 
Danach nur noch mit dem Bit in im Direction-Register spielen.

[Dennis T. (dennist92)]

Timings habe ich mit einem Logicanalyzer gecheckt, funktioniert 
eigentlich.

[Dennis T. (dennist92)]

I/O Funktionen sehen so aus.

void ds_out(){
  DS_DDR |= (1 << DS_PIN);
  DS_PORT |= (1 << DS_PIN);
}
void ds_in(){
  DS_DDR &= ~(1 << DS_PIN);
  DS_PORT |= (1 << DS_PIN);
}

[(prx) A. K. (prx)]

Lesen: Pin kurz aktiv auf Low ziehen, min 1µs max wenige µs, und nach 
15µs auslesen. Die 15µs sehe ich im Code nicht.

[(prx) A. K. (prx)]

Dennis T. schrieb:
> I/O Funktionen sehen so aus.

Ausser bei parasitärer Versorgung darf der Portpin nie auf High 
gezogen werden. Das macht der externe Pullup-Widerstand. Der interne 
Pullup ist zu hochohmig.

[Dennis T. (dennist92)]

das Auslesen klappt auch bei 1us, aber man sieht auch im Logicanalyzer, 
dass der falsche Werte gibt.

[(prx) A. K. (prx)]

Mit einem LA-Shot ohne Zeitachse kann ich nichts anfangen. Und wenn der 
Hersteller 15µs dranschreibt, dann verwendet man auch 15µs. Allerdings 
insgesamt, d.h. die unbekannte Laufzeit vom Code ist mit im Spiel.

[Dennis T. (dennist92)]

(prx) A. K. schrieb:
> Der interne
> Pullup ist zu hochohmig.
Also bei in und out die 2th Zeile entfernen?

[(prx) A. K. (prx)]

In dieser Phase ist übrigens ein Oszi besser als ein LA. Erst einmal 
sollten Timing und Pegel stimmen. Daten kommen später.

Was liest du, wenn nach Powerup des Sensors keine Messung erfolgt? Da 
müssen 85°C rauskommen.

[(prx) A. K. (prx)]

Dennis T. schrieb:
> Also bei in und out die 2th Zeile entfernen?

Korrekt.

[Dennis T. (dennist92)]

Habe ich noch nicht getestet, ohne Messung

[(prx) A. K. (prx)]

Dennis T. schrieb:
> das Auslesen klappt auch bei 1us, aber man sieht auch im Logicanalyzer,
> dass der falsche Werte gibt.

Findest du nicht, dass der Satz sich selbst widerspricht?

[Dennis T. (dennist92)]

Die Auslesezeit ist ja in erster Linie nicht wichtig, da ob ich auslese, 
oder nicht der DS die Werte überträgt, nun habe ich den internen Pullup 
weg gelassen, und der DS sendet keine ganzen Pakete mehr?

[Dennis T. (dennist92)]

Rot sind die Timings des DS

[Wolfgang (Gast)]

Dennis T. schrieb:
> Screenshot_2021-08-27_194122.png
> Rot sind die Timings des DS

Kannst du vielleicht mal eine ganz normale Zeitachse an das Zeitdiagramm 
vom LA packen oder die Daten wenigsten in nutzbarer Form anhängen?

Wer soll sich denn durch den Pixelsalat durchfummeln?

[Dennis T. (dennist92)]

Leider kann ich nicht alle Zeiten anzeigen lassen, sorry bin noch 
relativ neu in der Materie.

[Jack V. (jackv)]

Statt die Leute immer wieder mit unhandlichen und unnötigen Bildern zu 
beglücken, könntest du einfach die Aufnahme des LA in einem der dafür 
üblichen Formate speichern, und diese Datei mit den Daten hier anhängen. 
Da sind dann alle Informationen drin, und jeder kann sie sich so 
anschauen, wie er es braucht.

[Dennis T. (dennist92)]

Hier im CSV format

[(prx) A. K. (prx)]

Jack V. schrieb:
> Statt die Leute immer wieder mit unhandlichen und unnötigen Bildern zu
> beglücken

Was hast du gegen die linke Seite? Etwas zu viel Photoshop vielleicht.

[Stefan F. (Gast)]

Screenshots von einzelnen Fenstern macht man mit Strg-Druck. Oder mit 
dem Snipping Tool.

[MaWin (Gast)]

Dennis T. schrieb:
> ich eigentlich nur die letzten 2 Zeilen in ds_readtemp geändert

Du hast also das Programm aus einer anderen Quelle geholt, und dort 
funktioniert es (angeblich).
Dieselbe Taktfrequenz ?

Auf der einen Seite gibt es DS_PORT, auf der anderen PINC, ist DS_PORT 
auch PORTC, oder geändert ? (scheiss Definitionen).

[Dennis T. (dennist92)]

ne, die Lib habe ich selber erstellt, habe da keine gefunden.

// PIN
#define DS_DDR      DDRC
#define DS_PORT      PORTC
#define DS_PIN      0

[(prx) A. K. (prx)]

Dennis T. schrieb:
> ne, die Lib habe ich selber erstellt, habe da keine gefunden.

Beitrag "DS1820, DS18B20 in C"

[Falk B. (falk)]

Dennis T. schrieb:
> Leider kann ich nicht alle Zeiten anzeigen lassen, sorry bin noch
> relativ neu in der Materie.

Und in der Pupertät hängen geblieben ;-)

[Falk B. (falk)]

Dennis T. schrieb:
> ne, die Lib habe ich selber erstellt, habe da keine gefunden.

Ja klar, es hat ja auch noch niemals eine Onewire Bibliothek ins 
Internet gestellt. OMG!

Beitrag "Re: Onewire + DS18x20 Library"

Profitipp: Wenn man die CRC-Prüfung der Daten macht, erkennt man 
Übertragungsfehler.

[Dennis T. (dennist92)]

Falk B. schrieb:
> Und in der Pupertät hängen geblieben ;-)

und warum dies?

[Lutz (Gast)]

Weil du nebenbei ein Foto deiner viel älteren Schwester mitgeschickt 
hast...

[Lutz (Gast)]

Und nun schau dir endlich das 1Wire-Protokoll an. Wie ist ein 1 codiert, 
wie eine 0.

[Dennis T. (dennist92)]

Das Schaue ich mir später an, habe erst heute Abend wieder Zeit. Nein 
das ist nicht meine Schwester ;) Seltsamer Weise läuft der DS18 nun 
wieder.

[Wolfgang (Gast)]

Dennis T. schrieb:
> Seltsamer Weise läuft der DS18 nun wieder.

Steckbrettaufbau?

[Lutz (Gast)]

Dann macht _delay_us(1); definitiv nicht das, was man vermutet!

[Dennis T. (dennist92)]

Ich habe mal mein komplettes Projekt hier hoch geladen. es läuft Aktuell 
und zeigt die Temperatur an. Nicht über die F_CPU und die Delaywerte 
wundern, diese sind alle Kalibriert, klar wären Korrekte werte besser, 
habe aber irgendwelche Probleme mit den Fuses. Hier sind meine Fuses 
E:07, H:D9, L:57

[Falk B. (falk)]

Dennis T. schrieb:
> Ich habe mal mein komplettes Projekt hier hoch geladen. es läuft Aktuell
> und zeigt die Temperatur an. Nicht über die F_CPU und die Delaywerte
> wundern, diese sind alle Kalibriert, klar wären Korrekte werte besser,
> habe aber irgendwelche Probleme mit den Fuses. Hier sind meine Fuses
> E:07, H:D9, L:57

Naja, da gibt es "einiges" an Verbesserungspotential.

void ds_command(uint8_t command){
  int i;
  for (i = 0; i < 8; i++){
    ((command >> i) % 2) == 0 ? ds_writezero() : ds_writeone();
  }
}

Was soll das? Erstens ist eine variable Verschiebung  >> i relativ 
aufwändig und 2. sinnlos. Ein Modulo 2 ist es umso mehr. Akademischer 
und weltfremder geht es kaum. Das Ganze garniert mit dem ? Operator, den 
man so eher sparsam nutzen sollte. Ein schnödes if() tut es auch und ist 
besser lesbar.

Solche Sachen man man besser mit einer Maske und Bitmanipulation. 
Deutlich einfacher lesbar und deutlich performanter.

Deine Funktionen void ds_writezero(), void ds_writeone() ds_out() und 
ds_in() sind auch vollkommen akademisch aufgeblasen.

>Nicht über die F_CPU und die Delaywerte
>wundern, diese sind alle Kalibriert, klar wären Korrekte werte besser,
>habe aber irgendwelche Probleme mit den Fuses. Hier sind meine Fuses
>:07, H:D9, L:57

Dann löse zuerst diese Probleme, denn sonst wird dein Chaos noch größer!
Schreib ein Programm mit einem definierten Puls, z.B. 1ms, schau ihn dir 
mit dem Oszi oder Logikanaylsator an und stell deinen Fuses bzw. den 
RC-Oszillator so ein, daß er ausreichend genau 1ms erzeugt.
Hast du die Optimierung beim Compiler eingeschaltet? Ohne diese 
funktionieren diese Funktionen nicht!

[Dennis T. (dennist92)]

 for (i = 0; i < 8; i++){
    if((command >> i) % 2) == 0){
      ds_writezero();
    }else{
      ds_writeone();
    }
  }

und wie kann ich nun eine Maske und Bitmanipulation anwenden? habe nicht 
viel C Erfahrung, dazu kommt, dass die Digitaltechnik noch sehr neu ist. 
Es würde auch guter Lesestoff, oder Videos helfen. Dieses Thema schweift 
ja nun eigentlich sehr vom Topic ab :)

Danke euch übrigens sehr für eure Hilfe :)

[Stefan F. (Gast)]

Falk hat Recht. Zumindest bei AVR Mikrocontrollern sind Shift 
Operationen mit einer Variablen Anzahl von Schritten ziemlich 
ineffizient.

Vorher:

void ds_command(uint8_t command){
  int i;
  for (i = 0; i < 8; i++){
    ((command >> i) % 2) == 0 ? ds_writezero() : ds_writeone();
  }
}

Verbesserung:

void ds_command(uint8_t command){
  int i;
  for (i = 0; i < 8; i++){
    if (command & 1) {
      ds_writeone();
    }
    else {
      ds_writezero();
    }
    command = command >> 1;
  }
}

[Falk B. (falk)]

Dennis T. schrieb:
> und wie kann ich nun eine Maske und Bitmanipulation anwenden? habe nicht
> viel C Erfahrung, dazu kommt, dass die Digitaltechnik noch sehr neu ist.
> Es würde auch guter Lesestoff,

Ja, siehe Bitmanipulation bzw. mein Projekt.

https://www.mikrocontroller.net/topic/goto_post/4890827

> oder Videos helfen.

Nö. Du bist ein Millenial, welcher glaubt, die Welt kann vor allem mit 
"Tutorials" und Youtube-Videos erklärt werden. Dem ist nicht so. In 
vielen Fällen sind einfache Texte, sei es auf Papier oder elektronisch 
einem Video überlegen.

> Danke euch übrigens sehr für eure Hilfe :)

Was hast du gelernt? Wo lag der Fehler? Wie hast du ihn erkannt? Wie 
hast du ihn behoben?

[Vorn N. (eprofi)]

>for (i = 0; i < 8; i++){
>    if((command >> i) % 2) == 0){
>      ds_writezero();
>    }else{
>      ds_writeone();
>    }
>  }
Das geht eleganter (mask ist zugleich Bitmaske und Zähler):

mask = 0x80;do{
  if(command & mask) ds_writeone(); else ds:writezero();}
  while(mask/=2);

oder gleich:
mask = 0x80; do ds_write(command & mask) while(mask/=2);

> Es würde auch guter Lesestoff, oder Videos helfen.
Das Internet enthält genug gute Beispiele, aus denen man viel lernen 
kann.
Die Kunst ist, die Qualität der Beiträge zu bestimmen.

[Falk B. (falk)]

Vorn N. schrieb:
> Das geht eleganter (mask ist zugleich Bitmaske und Zähler):mask =
> 0x80;do{
>   if(command & mask) ds_writeone(); else ds:writezero();}
>   while(mask/=2);
> oder gleich:
> mask = 0x80; do ds_write(command & mask) while(mask/=2);

FAIL! OneWire sendet LSB first ;-)
Außerdem ist eine einfache UND-Verknüpfung und Auswertung kaum 
optimierbar, denn da kommt auf den meisten CPUs nur 1 ASM Befehl zum 
Bittesten raus.

"Ich bin dafür, die Dinge so weit wie möglich zu vereinfachen, aber 
nicht weiter."

Albert Einstein

[Stefan F. (Gast)]

Stefan ⛄ F. schrieb:
> if (command & 1)

Das ist die Maske, die Falk meint. Durch die UND-Verknüpfung wird nur 
das niedrigste Bit verwendet. Und wenn das 1 ist, dann wird der if-Block 
ausgeführt.

(Alle Ausdrücke die nicht 0 ergeben sind wahr)

> command = command >> 1;

Hier wird das Kommando bei jedem Schleifendurchlauf um eine Position 
nach rechts verschoben. Deswegen brauchen wir wir immer nur das gleiche 
Bit 0 mit if abzufragen. Die Verschiebung um 1 führt der Mikrocontroller 
besonders schnell aus, nämlich in nur einem einzigen Takt.

    if (command & 1) {
      ds_writeone();
    }
    else {
      ds_writezero();
    }

Alternative Schreibweise mit dem ternären Operator:

    (command & 1) ? ds_writeone() : ds_writezero();

Gefällt mir persönlich weniger gut, ist auch nicht effizienter.

[(prx) A. K. (prx)]

Vorn N. schrieb:
> mask/=2

Wenn man Bits schieben will, sollte man das auch tun, statt per 
Multiplikation oder Division um die Ecke zu denken. Zumal man dann je 
nach Datentypen und Intelligenz des Compilers Überraschungen erleben 
kann, nämlich wenn der Compiler mit Vorzeichen dividiert.

[Dennis T. (dennist92)]

/*
 * DS18B20.c
 *
 * Created: 27.07.2021 21:12:53
 *  Author: User
 */ 

#include <avr/io.h>
#include "../header/DS18B20.h"
#include <util/delay.h>

void ds_out(){
  DS_DDR |= (1 << DS_PIN);
}
void ds_in(){
  DS_DDR &= ~(1 << DS_PIN);
}

void ds_masterreset(){
  ds_out();
  DS_PORT &= ~(1 << DS_PIN);
  _delay_us(DS_MASTERRESET_US);
  ds_in();
  _delay_us(DS_WAIT_US);
}
void ds_command(uint8_t command){
  int i;
  
  for (i = 0; i < 8; i++){
    if (command & 1) {
      ds_writeone();
    }
    else {
      ds_writezero();
    }
    command = command >> 1;
  }
}
void ds_writezero(){
  ds_out();
  DS_PORT &= ~(1 << DS_PIN);
  _delay_us(DS_WRITEZERO_US);
  ds_in();
}
void ds_writeone(){
  ds_out();
  DS_PORT &= ~(1 << DS_PIN);
  ds_in();  
  _delay_us(DS_WRITEONE_US);
}
int ds_check(){
  ds_masterreset();
  ds_command(DS_SKIPROMCOMMAND);
  ds_command(DS_READSCRATCH);
  uint8_t temp1 = ds_readbyte();
  uint8_t temp2 = ds_readbyte();
  int out = ((temp2) * 256) + temp1;
  return out;
}
int ds_readtemp(){  
  ds_masterreset();
  ds_command(DS_SKIPROMCOMMAND);
  ds_command(DS_CONVERTTEMP);
  _delay_ms(DS_WAIT_MS);
  ds_masterreset();
  ds_command(DS_SKIPROMCOMMAND);
  ds_command(DS_READSCRATCH);
  uint8_t temp1 = ds_readbyte();
  uint8_t temp2 = ds_readbyte();
  int out = ((temp2) * 256) + temp1;
  return out;
}
int ds_readbit(){
  ds_out();
  DS_PORT &= ~(1 << DS_PIN);
  ds_in();
  _delay_us(30);
  if (PINC & (1 << DS_PIN)){
    return 1;
  }
  return 0;
} 
int ds_readbyte(){
  int i;
  int i_byte = 0;
  for (i = 0; i < 8; i++){
    i_byte += (ds_readbit() << i);
  }
  return i_byte;
}

Ich hab nun ein wenig bearbeitet, setze den internen Pullup nicht mehr 
auf high, sondern stelle den Port wieder auf Eingang. Das Timing passt 
nun auch, allerdings bei
#define F_CPU 2000000L obwohl ich ein 16mhz Quarz habe?
Zu der Frage woran der Fehler lag kann ich nichts sagen, denn ich habe 
zu der Zeit nur Code auskommentiert, hochgeladen und dann wieder zurück, 
ging dann irgendwie obwohl nichts anders war als vorher auch.

[Stefan F. (Gast)]

Dennis T. schrieb:
> for (i = 0; i < 8; i++){
>     i_byte += (ds_readbit() << i);
>   }

Hatten wir nicht gerade Shift Operationen mit variabler Schrittweite 
besprochen?

[Dennis T. (dennist92)]

Sehe ich als Verbesserungwürdig, mache ich gleich. Mal ne andere Frage 
habe einen usbasp für 10€ nutze atmel Studio und Abtdude. Atmel Studio 
läuft aber nur auf windoof, gibt es gute Alternativen?

[Dennis T. (dennist92)]

for (i = 0; i < 8; i++){ 
i_byte &= ds_readbit();
i_byte = i_byte >> 1;
 }

[Stefan F. (Gast)]

Dennis T. schrieb:
> Atmel Studio läuft aber nur auf windoof, gibt es gute Alternativen?

http://stefanfrings.de/avr_tools/index.html#ide

[Stefan F. (Gast)]

Dennis T. schrieb:
> for (i = 0; i < 8; i++){
>     i_byte &= ds_readbit();
>     i_byte = i_byte >> 1;
>  }

Ich würde mit i_byte=0 anfangen und dann |= anstelle von &= benutzen.

[Wolfgang (Gast)]

Dennis T. schrieb:
> #define F_CPU 2000000L obwohl ich ein 16mhz Quarz habe?

Und wird der externe Quarz auch benutzt (Fuses)?

[Stefan F. (Gast)]

Stefan ⛄ F. schrieb:
> Ich würde mit i_byte=0 anfangen und dann |= anstelle von &= benutzen.

Und nach links schieben! Nach rechts ist falsch.

[Dennis T. (dennist92)]

Dennis T. schrieb:
> Hier sind meine Fuses E:07, H:D9, L:57

Ist auf 16MHZ gestellt

Stefan ⛄ F. schrieb:
> Und nach links schieben! Nach rechts ist falsch.

Ja fiel mir auch nach dem Post ein, dass ich falschherum einlese. 
Momentan klagt aber ein anderes Problem, der DS antwortet mit FF.

[neuer PIC Freund (Gast)]

Fang erstmal ganz unten an. Deine ds_readbit() sind suboptimal. Sie 
verstoßen gegen die "Time Slot" Spezifikation des DS18B20, welche 
zwischen 60us und 120us liegen sollte. Du wartest nur bis zum 
Rückgabewert und brichst dann ab. Im rar 15us, oben 30us. Lass die 15 
und füge vor jedem return noch ein delay_us(70) ein.

[Stefan F. (Gast)]

Dennis T. schrieb:
> der DS antwortet mit FF.

Bzw. er antwortet gar nicht. Denn HIGH ist ja der Ruhepegel der Leitung.

[Joachim B. (jar)]

Falk B. schrieb:
> Das Ganze garniert mit dem ? Operator, den
> man so eher sparsam nutzen sollte. Ein schnödes if() tut es auch und ist
> besser lesbar.

da hast du Recht, aber da Monitore eher breit als hoch sind habe ich dem 
? Einzeiler mehr sichtbaren Quellcode, deswegen nutze ich den gerne.

[Stefan F. (Gast)]

(command & 1) ? ds_writeone() : ds_writezero();

Joachim B. schrieb:
> da Monitore eher breit als hoch sind habe ich dem
> ? Einzeiler mehr sichtbaren Quellcode, deswegen nutze ich den gerne.

Das:

if (command & 1) ds_writeone() else ds_writezero();

ist nicht verboten. Nur mal so als alternativen Vorschlag.

Ich benutze den ternären Operator gerne, wenn er einen Wert zurück 
liefert, wie in:

puts( (httpStatus>=400) ? "error" : "success" );

[Joachim B. (jar)]

Stefan ⛄ F. schrieb:
> if (command & 1) ds_writeone() else ds_writezero();

wetten da meckert der Compiler.

Stefan ⛄ F. schrieb:
> Ich benutze den ternären Operator gerne, wenn er einen Wert zurück
> liefert, wie in:puts( (httpStatus>=400) ? "error" : "success" );

me too

[Stefan F. (Gast)]

>> if (command & 1) ds_writeone() else ds_writezero();

Joachim B. schrieb:
> wetten da meckert der Compiler.

Sorry, da fehlt ein Semikolon hinter ds_writeone().

[Wolfgang (Gast)]

Dennis T. schrieb:
> Dennis T. schrieb:
>> Hier sind meine Fuses E:07, H:D9, L:57
>
> Ist auf 16MHZ gestellt

Ist er nicht. Mit den Einstellungen bekommt der uC einen Takt von 2MHz.
Guck dir mal das CKDIV8-Bit an.
https://eleccelerator.com/fusecalc/fusecalc.php?chip=atmega328p

[Stefan F. (Gast)]

Wolfgang schrieb:
> Ist er nicht. Mit den Einstellungen bekommt der uC einen Takt von 2MHz.

Es sei denn, sein Program setzt das CLKPR Register passend.

[Dennis T. (dennist92)]

Huch, oh man der hacken bei dividiere durch 8 ist ja drin 🤔. Dann passt 
ja doch alles :D

[Wolfgang (Gast)]

Stefan ⛄ F. schrieb:
> Es sei denn, sein Program setzt das CLKPR Register passend.

Unwahrscheinlich ;-)

Dennis T. schrieb:
> Studio_C.rar (75 KB)

Dennis T. schrieb:
> Das Timing passt nun auch, allerdings bei
> #define F_CPU 2000000L obwohl ich ein 16mhz Quarz habe?

[Dennis T. (dennist92)]

Hm, der DS gibt ein Presentpulse, aber ich kann ihn nicht mehr auslesen.

void ds_readscratch(){
  ds_masterreset();
  ds_command(DS_SKIPROMCOMMAND);
  ds_command(DS_READSCRATCH);
  for(int i = 0; i < 9; i++){
    scratch[i] = ds_readbyte();
  }
}

void ds_converttemp(){  
  ds_masterreset();
  ds_command(DS_SKIPROMCOMMAND);
  ds_command(DS_CONVERTTEMP);
  _delay_ms(DS_WAIT_MS);
}

int ds_readbit(){
  ds_out();
  DS_PORT &= ~(1 << DS_PIN);
  ds_in();
  if (PINC & (1 << DS_PIN)){
    _delay_us(60);
    return 1;
  }
  _delay_us(60);
  return 0;
} 
uint8_t ds_readbyte(){
  uint8_t i_byte = 0;
  for (int i = 0; i <  8; i++){
    i_byte |= ds_readbit();
    i_byte =  i_byte << 1;
  }
  return i_byte;
}

[Falk B. (falk)]

Dennis T. schrieb:
> Hm, der DS gibt ein Presentpulse, aber ich kann ihn nicht mehr auslesen.

Dein Quick & Dirty Hack ist Mist. Nicht rumstochern und "probieren", 
sondern ma nachdenken und auch MESSEN! Dein Lesen des Bits funktioniert 
so nicht. Man sollte schon die Zeiten einhalten. Und lass die Finger von 
dem PORT-Bit, das muss nur EINMAL auf 0 gesetzt werden, das macht 
eigentlich schon der Reset. Man muss nur die Richtung Eingang/Ausgang 
umschalten, denn es ist ein Open Drain Signal.

https://www.mikrocontroller.net/articles/Ausgangsstufen_Logik-ICs#Open_Collector

uint8_t ds_readbit(){
  uint8_t tmp=0;

  DS_DDR |= (1 << DS_PIN);   // LOW
  _delay_us(5);
  DS_DDR &= ~(1 << DS_PIN);  // Eingang/Tristate
  _delay_us(15);             // warten auf Reaktion
  if (PINC & (1 << DS_PIN)){ // lesen
    tmp = 1;
  }
  _delay_us(60);             // volle Bitzeit
  return tmp;
}

[Falk B. (falk)]

Dennis T. schrieb:
> uint8_t ds_readbyte(){
>   uint8_t i_byte = 0;
>   for (int i = 0; i <  8; i++){
>     i_byte |= ds_readbit();
>     i_byte =  i_byte << 1;
>   }
>   return i_byte;
> }

Und du hast es immer noch nicht verstanden, daß man das sinnvollerweise 
anders macht? Außerdem schiebst du falsch! OneWire überträgt LSB first, 
du machst aber aus dem 1. Bit das MSB durch dein nach links schieben! 
Außerdem schiebst du NACH der letzten Bitspeicherung, womit die Daten 
erst recht im Eimer sind! Eher so!

uint8_t ds_readbyte(){
  uint8_t i_byte = 0;

  for (int i = 0; i <  8; i++) {
    i_byte >>= 1;   // Platz schaffen für nächstes Bit ganz links
    if (ds_readbit()) i_byte |= (1<<7);
  }
  return i_byte;
}

[Dennis T. (dennist92)]

Falk B. schrieb:
> Dein Lesen des Bits funktioniert
> so nicht. Man sollte schon die Zeiten einhalten. Und lass die Finger von
> dem PORT-Bit, das muss nur EINMAL auf 0 gesetzt werden, das macht
> eigentlich schon der Reset

Das mit den Zeiten habe ich auch schon geregelt, habe ins Datenblatt und 
im Logic geschaut, dann fiel mir ein dass ich falsches Timing habe, 
würde vorher von geredet, nur vergessen umzusetzen.
Das einlesen habe ich nun auch geregelt, danke für eure Geduld :D mein 
Problem ist wirklich dass ich mehrere Fehler habe welche ich behebe, 
dadurch kommt anderes unter, ich aber selber auch suche, dadurch lerne 
ich es aber auch.
Ihr seit mir aber auch eine große Hilfe bin euch da sehr Dankbar :)

Das Thema des Topics ist nun eigentlich erfüllt, es läuft wie es soll.
alles weitere ist somit eigentlich nur noch Optimierung
Folglich Poste ich noch einmal die momentan endgültige Version.

Dann geht es erstmal zum nächsten Sensor dem DHT22 mal sehen was hier 
auf mich wartet :)

Nochmals großen Dank für die große Hilfsbereitschaft.

[Lutz (Gast)]

Falk B. schrieb:
> Was soll das? Erstens ist eine variable Verschiebung  >> i relativ
> aufwändig und 2. sinnlos.

Falk B. schrieb:
> Deine Funktionen void ds_writezero(), void ds_writeone() ds_out() und
> ds_in() sind auch vollkommen akademisch aufgeblasen.

Falk B. schrieb:
> uint8_t ds_readbyte(){
>   uint8_t i_byte = 0;
>   for (int i = 0; i <  8; i++) {
>     i_byte >>= 1;   // Platz schaffen für nächstes Bit ganz links
>     if (ds_readbit()) i_byte |= (1<<7);
>   }
>   return i_byte;
> }

Da predigt er von ganz weit oben (wo übrigens ziemlich wenig Sauerstoff 
ist...) einen runter, erwähnt Effizienz, zitiert sogar Onkel Albert usw. 
und kommt dann mit einer Schleife um die Ecke, die in jedem Durchlauf 
7 unnötige Shifts macht???

[Stefan F. (Gast)]

Lutz schrieb:
> und kommt dann mit einer Schleife um die Ecke, die in jedem Durchlauf
> 7 unnötige Shifts macht???

Jetzt bin ich aber sehr gespannt, wie du die 8 Bits ohne 8 
Shift-Operationen in ein Byte einliest.

An alle: Achtung, ich bitte um Aufmerksamkeit. Der Lutz zeigt uns allen 
jetzt einen genialen Trick den jeder Programmierer drauf haben sollte. 
Trommelwirbel bitte: taraa taraa taratatatataa

[(prx) A. K. (prx)]

Stefan ⛄ F. schrieb:
> Jetzt bin ich aber sehr gespannt, wie du die 8 Bits ohne 8
> Shift-Operationen in ein Byte einliest.

Überhaupt kein Problem und oben schon gezeigt. Man muss nur die Shifts 
durch Multiplikation und Division ersetzen. :-)

[Dennis T. (dennist92)]

Stefan ⛄ F. schrieb:
> Jetzt bin ich aber sehr gespannt, wie du die 8 Bits ohne 8
> Shift-Operationen in ein Byte einliest.

Die 7 durch i ersetzen

uint8_t ds_readbyte(){
  uint8_t i_byte = 0;
  for (int i = 0; i <  8; i++) {
    if (ds_readbit()) i_byte |= (1<<i);
  }
  return i_byte;
}

[STK500-Besitzer (Gast)]

Dennis T. schrieb:
> Die 7 durch i ersetzen

LOL
Was macht der Compiler wohl aus "1<<i"?

Es gibt Dinge, die man nicht besser machen kann.

[(prx) A. K. (prx)]

STK500-Besitzer schrieb:
> Was macht der Compiler wohl aus "1<<i"?

Eine Konstante, wenn du ihn dazu kriegst, die Schleife zu unrollen. :-)

Wobei man das auch selber machen kann. 8x der gleiche Code statt 
Schleife, und weg ist der Shift.

[STK500-Besitzer (Gast)]

(prx) A. K. schrieb:
> Wobei man das auch selber machen kann. 8x der gleiche Code statt
> Schleife, und weg ist der Shift.

OK...

[Dennis T. (dennist92)]

(prx) A. K. schrieb:
> Wobei man das auch selber machen kann. 8x der gleiche Code statt
> Schleife, und weg ist der Shift.

Ist wohl längerer Code, aber nicht zu viel und nach dem compilieren 
wahrscheinlich sogar effektiver und kleiner, als mit Schleife und 
variable?

[Lutz (Gast)]

Stefan ⛄ F. schrieb:
> Lutz schrieb:
>> und kommt dann mit einer Schleife um die Ecke, die in jedem Durchlauf
>> 7 unnötige Shifts macht???
>
> Jetzt bin ich aber sehr gespannt, wie du die 8 Bits ohne 8
> Shift-Operationen in ein Byte einliest.

Oh man, mal wieder der Beweis, daß es der Original-Stefan (nicht 
gehackter Account) ist...

Ich schrieb etwas von 7, du etwas von 8. Komm; das schaffst du im 
zweiten Versuch...
Ich muß jetzt leider mit dem Kochen anfangen und kann dich daher erstmal 
nicht weiter beaufsichtigen.

[Lutz (Gast)]

Nun gut, Kommando zurück: 1<<7 ist ja eine Konstante zum verODERn. Das 
wird ja nicht zur Laufzeit geshiftet.

[Stefan F. (Gast)]

Lutz schrieb:
> Nun gut, Kommando zurück: 1<<7 ist ja eine Konstante zum verODERn.
> Das wird ja nicht zur Laufzeit geshiftet.

Na also, geht doch. Ich hoffe du schämst dich jetzt ein bisschen für 
deine persönlichen Angriffe gegen Falk und mich.

Lutz schrieb:
> Ich schrieb etwas von 7, du etwas von 8.

Aus gutem Grund, denn die Beispiele von Falk und mir nutzen 8 
Schiebe-Operationen (einer mehr als nötig, OK). Du hast geschrieben, 
dass davon 7 Shifts unnötig seien. Man hätte demnach also alle 8 Bits 
mit nur einer Shift Operation einlesen müssen.