amrij / zonnepanelen

Website which shows telemetry data from the TCP traffic of SolarEdge PV inverters
GNU General Public License v3.0
12 stars 6 forks source link

Parse error #64

Closed jvanderzande closed 5 years ago

jvanderzande commented 5 years ago

Ik krijg met deze laatste Development versie de volgende error: "PHP Parse error: syntax error, unexpected 'case' (T_CASE), expecting end of file in ....\live-server-data-10sec.php on line 500". Werkt die gewoon voor jullie?

Jos

jvanderzande commented 5 years ago

Tweede regel is het probleem. een "}" voor de if.

case "2": // DSMR
        } if ($period == 'm') {
amrij commented 5 years ago

Ik heb fout in case "2" gecorrigeerd in development. Werkt het nu wel?

amrij commented 5 years ago

Er was nog een fout geïntroduceerd met de laatste aanpassingen van Marcel Mol. Ik heb dit teruggedraaid en de regels case "2": // DSMR } if ($period == 'm') { if($limit == '') { $limit = '12'; } $periods = $limit31-1; } else { if($period == '' || $period == 'd' ) { $SQLdatefilter = '"%Y-%m-%d"'; $JSON_SUM = "Y-m-d"; $JSON_period = "day"; if($limit == '') { $limit = '30'; } $periods = $limit-1; } elseif ($period == 'm') { $SQLdatefilter = '"%Y-%m"'; $JSON_SUM = "Y-m"; $JSON_period = "month"; if($limit == '') { $limit = '12'; } $periods = $limit31-1; } veranderd in: case "2": // DSMR if($period == '' || $period == 'd' ) { $SQLdatefilter = '"%Y-%m-%d"'; $JSON_SUM = "Y-m-d"; $JSON_period = "day"; if($limit == '') { $limit = '30'; } $periods = $limit-1; } elseif ($period == 'm') { $SQLdatefilter = '"%Y-%m"'; $JSON_SUM = "Y-m"; $JSON_period = "month"; if($limit == '') { $limit = '12'; } $periods = $limit*31-1; }

Het geeft nu bij mij geen foutmeldingen meer.

Marcel, kun je nog een keer de wijzigingen aanbrengen op de laatste files?

mirakels commented 5 years ago

Ik denk dat de oplossing van Jos voldoende zou moeten zijn: de '}' weghalen.

De selectie voor limiet en period zou in theory voor alle case-es hetzelfde moeten zijn, maar was voor alle drie min of meer verschillend. Ik poogde dus om te beginnen alle common code naar boven te halen en samen met jullie te kijken of de overige verschillen per case 'weg te werken'...

amrij commented 5 years ago

Zonder de aanpassing van Jos en na jouw aanpassing kreeg ik alleen nog de waardes van één dag. Na de aanpassing van Jos werkte het niet meer, ook bij de versie van met jouw aanpassing. Na weghalen van de } en } else { werkte de 1e versie maar niet met de aanpassingen van jouw. De } else { werd niet afgesloten met een } Het aantal } en { was gelijk. waarschijnlijk om die reden heeft het bij de 1e versie geen foutmelding gegeven. door één } weg te halen ging het fout. Bij nadere bestudering bleek if ($period == 'm') twee keer achter elkaar er in te staan. Daarom heb ik dit in het geheel weggehaald.

Mijn idee was in eerste instantie om de verschillen weg te werken, maar dat was moeilijker dan gedacht. Daarom heb ik voor deze oplossing gekozen.

mirakels commented 5 years ago

Andre, het is nu een beetje een puinhoop met reverts op reverts. Ik zie nu dat bvb de short temp patch half is dooregvoerd, net zoals de ServerTime naar ts change.

Wat misschien het handigste is, is om alles te forceren naar net voor mijn laatste pull request. Ik heb op mijn 'mistral' repository nu het volgende gedaan:

$ git reset --hard c1d44c3952fe47add3e26e35677ce50a12a7d164
$ git push --force

(vanaf een linux prompt. Ik denk dat jij vanuit windows werkt, maar daar zal hetzelfde mogelijk moeten zijn). Op deze manier wordt de vervuiling weggewerkt en omdat we in princiepe toch maar met z'n 3-en op de repo werken moet dat verder niet zo'n probleem zijn.

Dan stuur ik daarna een kort pullrequest waar die kleien wijzigignine in zitten, en kunnen we daarna de 'common code' patch aanpakken (ik heb denk ik nu al een goede klaarstaan)

amrij commented 5 years ago

Ik heb bovenstaande comondo's uitgevoerd.

De oorspronkelijke file was goed. De fout is ontstaan bij: mirakels added some commits 4 days ago @mirakels Shorten metric name for temperature 0e6369e @mirakels P1 current: Pass timestamp as int instead of string … 3d398e9 @mirakels live-data 10 sec: cosmetic … f4ba821 @mirakels live 10sec: reformat P1_Meter sql queries 0a8ced5

live-data 10 sec: cosmetic

bij case: "2" is het volgende uitgevoerd:

image

Zie bij de historie van de Pull requests.

Ik had deze aanpassingen niet gecontroleerd voor de merged commit omdat tot nog toe er tekens geen problemen waren gevonden bij de controle en ik ervan uitging dat de aanpassingen getest zijn.

Door deze aanpassing krijg ik alleen de gegevens van één dag en niet de historie.

Dit is een harde les voor me.

Ik wacht je pull request af, Je moet wel de laatste versie van zonnepanelen.php gebruiken waarbij bij alle ajax requests cache: false is opgenomen.

mirakels commented 5 years ago

Ik had al gewaarschud in de pull request, om eerst even te testen (ik heb geen dommotics of dmsr, gebruik gewoon de P! tabel in dataabse). Maar er zaten wel stomme fouten in die wel had moeten zien.

amrij commented 5 years ago

Ik heb ook geen dommotics of dmsr en heb voor de eerste versie eerst aan Jos gevraagd of hij het wilde testen. Hij had geen fouten gevonden. Je moet dus voorzichtig zijn om codes te wijzigen die je niet kunt testen. Bij de eerste versie heb ik alleen de codes samengevoegd zonder iets aan de codes te wijzigen. Feit blijft dat de fout ook invloed had op de P1 versie.

mirakels commented 5 years ago

Klopt. ik had deze te snel gepushed.

De nieuwe pull request bevat alleen de 'simpele' delen. Alleen de ServerTime -> ts heeft impact op domitcz en dsmr die ik niet kan testen. P1 database heb ik wel getest en werkt prima hier.

amrij commented 5 years ago

Wat is het nut van deze aanpassing? Vooral als je het niet zelf kunt testen is dit niet zinvol dit door te voeren.

mirakels commented 5 years ago

Het nut staat in de commit log.... Bij mij liep de 'electiciteitsgebruik vandaag' steeds 2 uur achter. Het bleek dat het getoonde tijdstip niet klopte. het tijdstip dat daarvoor gebruikt wordt is de 'ServerTime'. Dit is was een string in de vorm "2019-07-15 14:15:15" waaruit in zonnepanelen.php de uren:minuten uitgehaald worden. Nadat ik ServerTime had omgezet van een tijdstring naar een unixtimestamp werkte het wel goed.

Maar ik kan niet alleen voor P1 databse die wijziging doorvoeren want dan gaan domotics en dsmr de mist in. Dus moet ik ze voor alle drie aanpassen.

Ik dacht dat er een development tree was om wijzigingen te kunnen testen voordat ze naar master gaan en dat dat juist de mogelijkheid biedt om elkaar te vragen om iets te testen. Als er geen wijzigingen gemaakt mogen worden in delen die wel gerelateerd zijn aan elkaar maar die je niet kan testen, dan kunnen we wel ophouden met developen...

amrij commented 5 years ago

Ik gebruik ook P1 en bij mij treedt die fout niet op. Indien je een fout constateert kun je uitleggen wat de fout is en daarvoor een oplossing voorstellen. De fout kan optreden omdat je bijvoorbeeld op een ander platform werkt.

Indien een programma goed loopt is development niet bedoelt om cosmetische wijzigingen door te voeren waarbij het risico van fouten aanwezig is. Verandering van code is alleen bedoeld om bugs te verwijderen, functies toe te voegen of de snelheid te vergroten. Het vraagt veel tijd van iedereen om te testen van niet functionele wijzigingen.

mirakels commented 5 years ago

ok duidelijk