Hey,
Ik heb voor de opendag van mijn opleiding een extratje gedaan en een schakeling gemaakt waarop 6 digits en 5 LED's worden aangestuurd door een Arduino mega 2560. Nu heb ik een code ervoor geschreven die redelijk lang is en ik heb zo een gevoel dat er meerdere dingen zijn die niet persee simpeler, maar vooral compacter en beter kunnen. Dus nu wil ik aan de experts die op dit forum kijken vragen of ze mijn code willen doorkijken en feedback kunnen geven wat ik in de toekomst beter kan doen!
Alvast bedankt!
Even een uitleg over de code (Wat het doet) ik print 5 woorden achter elkaar uit en daarna komt een overgang waarbij de displays worden gevuld met achten, ik gebruik commA t/m commF die elke digit apart aanzet via de common anode M.b.v. transistoren (zodat de chip niet de stroom levert maar de voeding zelf. Was het idee erbij) en de pinnen 30 t/m 36 gebruikt ik als sink voor de output als de pin laag is (segment aan) en als de pin hoog is (segment uit). en De 5 extra leds leek het me leuk om een wave van te maken die fade. Ik heb zelf zoveel mogelijk commentaar in de code geschreven zolang dit mij praktisch leek, als iets niet duidelijk is hoor ik het graag! Hier komt de code dan 
EDIT: Commentaar kan captain obvious zijn omdat ik verwacht dat op de opendag nog niet veel mensen weten hoe programmeren werkt
EDIT: code was meer dan 9000 characters dus heb een link: Code LBucks - Pastebin.com
en de fotos te groot dus nog een link!: http://1drv.ms/1Apga53
Netjes geprogrammeerd.!!!
a) Zelf zou ik het aansturen van de commA t/m commF in een tabelletje gezet hebben omdat je ze toch sequentieel afloopt.
a1) in het opdelen van jouw programma kun je het beste nastreven om alles maar 1x te moeten schrijven. Als code stukken hetzelfde zijn kun je overwegen om er een functie van te maken. Eventueel met parameters.
b) Zaken als // BEGIN IF en // END IF zou ik zelf nooit gebruiken. Goed inspringen
c) als je nog met de Arduino IDE werkt zou ik daar heel gauw mee stoppen met grotere programma's en lekker met de Eclipse + Jantje;s plugin werken. Als je dat in combinatie gebruikt met Doxygen kun je uit je programma een complete website genereren (zie als voorbeeld: malawi.verelec.nl).
d) Verder ben ik een grote zeikerd als komt op netjes coderen... = tekens onder elkaar etc. Bekijk de source maar. Die staat ook op de site (alleen zonder de multiline comments).
Ik heb de originele source code hier ff toegevoegd als attachment.
Malawi_001.ino (10.5 KB)
Bedankt voor de tips!
Ik zie wat je bedoeld met netter coderen zonder //endif etc. gewoon wat vaker op tab duwen maakt jouw code een stuk duidelijker te lezen ivm mijn code!
Echter je tip met het sequentieel definen begrijp ik niet precies hoe ik het dan zou moeten doen... Zo als ik het nu heb begrepen lijkt het mij niet simpeler om de 6 defines in een functie apart te zetten. En ik weet iets af van een #ifndef, en in de code van een college iets gezien hoe hij met een struct werkt?!? Ik heb verder nog niet echt een grote kennis van alle opties is C. Dit is zelfs pas mijn aller eerste eigen geschreven code voor een arduino. Multiplexing en de andere methodes heb ik pas sinds een paar weken geleden geleerd toen ik een PIC18F4480 moest leren programmeren als interne stage opdracht 
EDIT: en ja ik werk nog met de arduino IDE en vond het al een beetje irritant te worden met deze lengte aan code. Ben al aant kijken naar vervangers!
voorbeeld:
uint8_t common[6]; // common aanslutingen van de leds
initialiseren:
for (uint8_t i = 0; i < sizeof(common); i++){
pinMode(common[i], OUTPUT);
}
uitzetten:
for (uint8_t i = 0; i < sizeof(common); i++){
digitalWrite(common[i], HIGH);
}
aanzetten zoals in jouw code:
digitalWrite(common[ digitswitch], LOW);
En dan kan die hele switch case eruit.
Ik ben het eens met Nico dat het netjes geprogrammeerd is.
Ik heb toch een aanmerking over de wijze van comments die je hebt gemaakt.
Aan de ene kant geef je overvloedig, soms zelfs overbodige comments waar een if, for en switch beginnen en eindigen maar bij andere essentiele zaken die eigenlijk wel uitleg benodigen ontbreekt die.
Neem nu het gebruik van DDR (Data Direction Register) A en C, heel compact om dat zo te gebruiken maar je hebt in je Mega-2560 128 kB 256kB programmeer geheugen tot je beschikking en je gebruikt daar nog geen 3kB van. Dit compacte coderen maakt je code voor beginners ondoorgrondelijk, zeker omdat je hierbij nauwelijks uitleg geeft.
Om dan maar even te zeuren, ik vind het gebruik van DDRA en DDRC hier niet op zijn plaats. Ik neem aan, gezien dat je je code hier op dit forum plaatst, dat iedereen met alleen basis Arduino programmeer kennis je code moet kunnen begrijpen. Jouw code vereist diepere kennis van de hardware van de AVR processor architectuur. Foei ! 
cartoonist:
ik vind het gebruik van DDRA en DDRC hier niet op zijn plaats. Ik neem aan, gezien dat je je code hier op dit forum plaatst, dat iedereen met alleen basis Arduino programmeer kennis je code moet kunnen begrijpen. Jouw code vereistdieperekennis van de hardware van de AVR processor architectuur. Foei !
Wel een heel bijzondere vreemde manier van redeneren.....
Nico, als ervaren C++ programmeur kunt jij je misschien wel niet meer verplaatsen in de wereld van de beginnende Arduino programmeur. De beginner wil geen slimmigheidjes zien op dit forum maar duidelijk gedocumenteerde voorbeelden die zich liefst ook aan de referenties van de arduino taal houden. Duidelijkheid voor alles.
Nogmaals er is , afgezien van de zeer gebrekkige documentatie, niets mis met het gepresenteerde programma.
Hinderlijk is ook dat de ducumentatie bij de definities van de letters afwezig is. De maker heeft zijn eigen volgorde van codering van de A,B,C,D,E,F, G en DP segmanten aangehouden zonder enige documentatie.
Dit maakt het zeer moeilijk voor derden om dit programma te gebruiken met een andere tekst op de diplays.
Wat betreft de documentatie krijgt de maker van dit programma van mij een dikke onvoldoende
cartoonist:
Nico, als ervaren C++ programmeur kunt jij je misschien wel niet meer verplaatsen in de wereld van de beginnende Arduino programmeur. De beginner wil geen slimmigheidjes zien op dit forum maar duidelijk gedocumenteerde voorbeelden die zich liefst ook aan de referenties van de arduino taal houden. Duidelijkheid voor alles.
Ik wist niet dat dit forum alleen voor beginners was? En des te verbaasder ben ik dat jij nu meent te moeten spreken voor de beginnende Arduino programmeur. Of ben jij tegenwoordig de moderator voor dit forum geworden? Tja ik weet het niet hoor? Kan best zijn dat er wat gewijzigd is in het beleid en dat ik dan maar mijn heil elders moet zoeken? Wat je wilt.....
We zullen maar afwachten of er nog anderen forumleden zijn die een mening hierover hebben.
Mijn mening weet je ondertussen.
Als moderator kan ik hier heel formeel zijn.
Het product Arduino richt zich (in de eerste plaats) tot de beginner en de artiest.
Het forum staat voor kennis verbreding en verspreiding. Dat beperkt zich dus niet alleen tot beginners.
Zo heel diepgaande discussies over AVR, SAM, C, C++ enzovoort zijn -indien ze in verband staan met Arduino- even welkom als een beginners vraag.
Merk op dat het forum gemonitord en beheerd word door vrijwilligers. Allemaal mensen met diepe wortels in een van de Arduino gebieden (electronica, electriciteit, mechanica, software). Voor hen is een diepgaandere vraag juist een reden om vol te houden.
Met vriendelijke groet
Jantje
Nu ook even mijn persoonlijke mening.
Het is uit de vraag -en zeker de bijgeleverde code- heel duidelijk dat OP wel van wanten weet wat betreft codering.
Nico zijn opmerkingen zijn goed ontvangen door OP en OP vraagt meer uitleg.
Nico levert die uitleg.
cartoonist start eeb discussie over "hoe een antwoord er hoort uit te zien"
OP heeft geen enkel belang bij een discussie of de code al dan niet "volgens de regels van cartoonist" zijn. Ook als forum hebben we geen enkel voordeel om deze discussie in deze post te voeren.
Moesten je opmerkingen naar mij gericht zijn; ik zou me beledigd voelen. Daarom beschouw ik je opmerkingen als beledigend wat ongepast is.
Dus volgende keer als je vindt dat iemand "buiten de lijntjes kleurt" raad ik je aan een van volgende acties te ondernemen
- selecteer report to moderator en leg uit wat er volgens jou fout gaat.
- Start een nieuwe post en "leg je probleem uit en vraag input"
Met vriendelijke groet
Jantje
@ moderator
Topic starter vraagt niet alleen om uitleg maar ook om feedback over wat hij in de toekomst beter kan doen.Onder "good code practice" moet ook voldoende documentatie of verduidelijkende comments worden inbegrepen. Mijn kritiek (met een knipoog) is daarom gericht op het gemis van comments op enkele onduidelijke maar wel essentiele onderdelen van de code.
Wat betreft meningen, ik neem aan dat, ook op dit forum, het iedereen vrij staat om een, terzake doende, mening te hebben en die te onderbouwen. Ik 'spreek' hier op dit forum enkel voor mijzelf en niet voor of namens 'de beginner'. In mijn posts richt ik mij wel meer tot de beginner omdat ik zie dat er in het NL forum heel veel beginners vertoeven, daarom bekritiseer ik wel eens forumleden die, naar het lijkt, dat punt soms vergeten. Die kritiek is nooit kwetsend of beledigend bedoeld, wel vaak scherp. Anderen doen het op hun eigen manier en ik probeer daar altijd voldoende respect voor te hebben.
Met vriendelijke en respectvolle groet,
Cartoonist.
@ L_Bucks
while(setledmode <= 13)
{
pinMode (setledmode, OUTPUT); //Ik wilde van pin 0 en 1 afblijven omdat het communicatie poorten waren
setledmode ++;
}
Waarom heb je hier niet DDRK = 0b11111111; gebruikt ? // PORT K als OUTPUT
en dan de 6 leds aangesloten op pin 8 t/m 13.
Daar ga ik even fout. PORT K is Analog In en niet PWM.
PORT B = 10,11,12,13 en 52,53. helaas zijn 52 en 53 geen PWM outputs dus zoeken naar een andere poort.
Dan maar DDRH erbij gebruiken
DDRB = 0b11111111;
DDRH = 0b11111111;
en dan de Leds op 8,9,10,11,12,13
Het is wel logischer (en overzichtelijker ?) om alle pinMode's op dezelfde manier te initialiseren.
Compacter en consequent alles met PORT registers en alleen die pinnen die ook als OUTPUT worden gebruikt initialiseren:
// Als referentie zie de info onderaan http://arduino-info.wikispaces.com/MegaQuickRef
void setup() {
//0 = input 1 = output
DDRA = 0b11111100; //COMM poort pin 24,25,26,27,28 en 29 als OUTPUT
DDRC = 0b11111111; //SEG poort alle pinnen OUTPUT
DDRB = 0b11110000; // pinnen 10,11,12,13 als PWM OUTPUT
DDRH = 0b01100000; // pin 8 en 9 als PWM OUTPUT
}
De helft met DDRx en de andere helft met pinMode initialiseren is half werk 
Allereerst jij krijgt van mij een dikke 9 voor je werk.
Welke opleiding doe je?
Laat die oude rotten maar zeuren (shit ben 16+40)
in de IDE zit een automatische format dan staan alle haakjes precies daar waar je ze wilt hebben.
Jij bent er natuurlijk ook wel achter dat er een library voor bestaat, maar ik vind het prachtig dat je het zelf uitgezocht hebt.
Goed documenteren is vaak een probleem, maar dit ziet er prachtig uit. (ik doe ook // end if schrijven (kleine letters) ik zet er dan wel achter waarvan, zeker als het wat verder naar boven is.
DDRx gebruiken is prima, wel lastig voor beginners, maar ach het is wel een stuk sneller.
Heb je ook al een schema gemaakt (fritzing)
regel 135 is FOUT 
Je kunt een functie getallen meegeven, dan kun je dus zeggen zet digit(x) aan.
Kijk eens hoe locale variabelen werken nu staat alles global.
Mijn zoon zegt dat het harde code is, daar heeft hij gelijk in, dat maakt de code heel moeilijk leesbaar en wijzigbaar, hij bedoelt dat de letter bijvoorbeeld a varieert qua opbouw.
je kunt beter een tabel maken met alle 26 letters en (zie het papier onder de fotos
en dan kun je de display van daar uit sturen.
letter [65]=0b11101110 // a een soort ascii tabel dus. of is het 0b00001000
letter [66] = 0b10100000 enz.
tekst ="hallo en welkom bij summa" dan kun je de tekst mooi laten lopen
en natuurlijk heel eenvoudig wijzigen
dus display[1] = tekst[1]
display[2]=tekst[2]
je hebt 6 displays
dan kun je de tekst gewoon als een tekst neerzetten, zonder dat je iedere keer moet kijken hoe een letter eruit ziet.
mooie opgave voor examen)
tellertje eraan welke letter aan de beurt is.
Ik neem wel mijn petje af voor het parrallel computing wat je doet met die tijden prachtig (ja ik weet het is standaard theorie)
genoeg voor vandaag
Hallo allemaal,
@niceverduin:
wow... Dat is zo logisch dat ik mij zelf even een facepalm moest geven ;p Bedankt voor het opmerken!
@cartoonist:
Ik ben het wel eens met je opmerking over de overbodige //begin if en //end if. Persoonlijk vond ik het al niet optimaal en @nicoverduin doet het naar mijn mening overzichtelijker door gewoon meer uit te springen bij een statement.
Nog iets; ik doe het liever per poort initialiseren. Ik vindt dit persoonlijk duidelijker omdat ik dit als eerste leerde.. Met een PIC18f4480 initialiseer je de pinnen namelijk met een TRIS[Poort letter] register. En vandaar dat ik dit het liefste deed. Echter is er geen manier om een pin/bit te negeren als je de poort initialiseert? Bijv: DDRA bit 7 en 6 negeren kun je ze gewoon weg laten: DDRA = 0b111111 want je leest de bits van rechts naar links, dus bit 0 begint het rechts mee en dan tot bit 5. Maar wat als ik nu bit 0 en bit 2 wil negeren (en ik weet de default richting van de pin niet)? Ik ben nu op stage dus zou vanavond pas weer kunnen testen, maar ik bedenkt me nu zoiets als DDRA = 0b11111x1x?!? Of herkent de compiler dit niet?
@shooter
thanks!
I volg MBO mechatronica niveau 4 op het Summa in Eindhoven! Leerjaar 2.
Bedoel je met automatische format gewoon tab spammen, of is dit een knop in het menu? (Heb op dit moment geen toegang tot de IDE.. Stage beveiligde laptops ;(
Maar uhmmm library voor de positionering van de haakjes?!?!!? Please tell me more
Regel 135 is idd een copy paste foutje.. Mijn excuses.
Om een digit aan te zetten moet de pin gelinkt aan het segment laag zijn, want ik gebruik het als sink.
En het idee van elke letter in het programma gooien.. bedoel je dus zo:
letter [26] {alle letters achter elkaar in bits...};
#define a letter[0]
#define b letter[1]
#define c letter[2]
etc...
//dit snap ik nog, makkelijker kiezen welke letter, maar dan??:
//als ik nu acb wil laten zien op het display?
//Nu kan ik geen for statement gebruiken want het is niet simpel +1 de heletijd
//dus moet ik nu een switch statement in elke case van een switch statement plaatsen?!?!
//hier vindt ik het wazig worden dus extra uitleg please
PS: houd er wel rekening mee dat ik multiplexing gebruik, dus value[1] moet samen met letter [1] zijn
WOOPS, snapte het net optijd DUS:
op het einde dan één switch statement met in de case:
case 0:
value[0] = a;
value[1] = c;
value[2] = b;
break;
enzzz...
toch?
parrallel computing? Wist niet dat er een definitie voor was, maar bedankt!
Echter is er geen manier om een pin/bit te negeren als je de poort initialiseert? Bijv: DDRA bit 7 en 6 negeren kun je ze gewoon weg laten: DDRA = 0b111111 want je leest de bits van rechts naar links, dus bit 0 begint het rechts mee en dan tot bit 5. Maar wat als ik nu bit 0 en bit 2 wil negeren (en ik weet de default richting van de pin niet)? Ik ben nu op stage dus zou vanavond pas weer kunnen testen, maar ik bedenkt me nu zoiets als DDRA = 0b11111x1x?!? Of herkent de compiler dit niet?
Die is er wel en dit staat hier PORT Manipulation duidelijk uitgelegd.
DDRD = DDRD | B11111100; // this is safer as it sets pins 2 to 7 as outputs
// without changing the value of pins 0 & 1, which are RX & TX
Met de Bitwise OR wordt daar waar een 0 staat de oude toestand niet gewijzigd, daar waar een 1 staat wordt die poort als OUTPUT gedefinieerd.
Wat betreft de segment codering:
De algemeen gehanteerde vorm is om de volgorde DP, G,F,E,D,C,B,A aan te houden en de segmenten tel je van boven, de A, rechtsom tot 'F' , de middelste is dan 'G'. De DP staat helemaal links als MSB.
Als je die volgorde had gebruikt had ik, als ik dat zou willen, gemakkelijk jouw tekst kunnen vervangen door mijn eigen tekst.
format is gewoon in het menu, dus simpel houden.
als je bijvoorbeel NPP gebruikt dan kun je de taal instellen op C dan krijg je ook gelijk mooie indenten.
ja die defines kun je ook maken maar dat is niet echt nodig.
in ascii is 65 een A
dus als je vraagt welke code heeft de volgende letter dan haalt hij gelijk de goede bitjes op.
om het makkelijk te maken maak je dus een array aan van 65 tot 65+26
je hebt een stuk tekst "HALLO WELKOM BIJ SUMMA"
for teller1 to tekstlengte
for displayteller 1 to 5
letter= tekst[teller+displayteller]
output display[1]=alfabet[letter]
end for
end for
zo ongeveer dus,
ik woon in shertogenbosch, en kom graag op de open dag kijken hoor,