fanspeed-comparator #19

Closed
Athamantis wants to merge 0 commits from fanspeed-comparator into main
Owner

compare a sett value to two fans

compare a sett value to two fans
Athamantis added 39 commits 2024-04-27 15:32:45 +00:00
Athamantis requested review from sebgab 2024-04-27 15:32:54 +00:00
Athamantis requested review from Inamr 2024-04-27 15:32:54 +00:00
sebgab requested changes 2024-04-27 15:43:25 +00:00
sebgab left a comment
Owner

Just a couple things

Just a couple things
@ -0,0 +1,113 @@
//#include <avr/ioavr128db48.h>
Owner

Remove commented out include.

Remove commented out include.
Author
Owner
ac5a054f8b6e04b06183bea9d740d63066d6ef22
Athamantis marked this conversation as resolved
@ -0,0 +4,4 @@
#include "uart.h"
uint16_t timer_period_ms = 1;
uint16_t fan_speed;
Owner

Variable should be initialized to a value.
e.g. uint16_t fan_speed = 0;

Variable should be initialized to a value. e.g. `uint16_t fan_speed = 0;`
Author
Owner
ac5a054f8b6e04b06183bea9d740d63066d6ef22
Athamantis marked this conversation as resolved
@ -0,0 +7,4 @@
uint16_t fan_speed;
volatile uint16_t falling_edge_counter_1 = 0;
volatile uint16_t falling_edge_counter_2 = 0;
#define MAX_PER 0x50
Owner

Remove unused define

Remove unused define
Author
Owner
ac5a054f8b6e04b06183bea9d740d63066d6ef22
Athamantis marked this conversation as resolved
@ -0,0 +14,4 @@
TCA0.SINGLE.INTCTRL = TCA_SINGLE_OVF_bm ;
TCA0.SINGLE.CTRLA = TCA_SINGLE_ENABLE_bm | TCA_SINGLE_CLKSEL_DIV1024_gc ; /* Sysclk /1024 */
TCA0.SINGLE.PERBUF = 3906;
//TCA0_update_period_ms();
Owner

The function should be used based on the variable set earlier, rather than the perbuf being hardcoded.

The function should be used based on the variable set earlier, rather than the perbuf being hardcoded.
Author
Owner
ac5a054f8b6e04b06183bea9d740d63066d6ef22
Athamantis marked this conversation as resolved
@ -0,0 +17,4 @@
//TCA0_update_period_ms();
}
void TCA0_update_period_ms() {
Owner

This function should be used and it should calculate the ms with the formula from the øving.

This function should be used and it should calculate the ms with the formula from the øving.
Author
Owner
ac5a054f8b6e04b06183bea9d740d63066d6ef22
Athamantis marked this conversation as resolved
@ -0,0 +21,4 @@
TCA0.SINGLE.PERBUF = (F_CPU * (1 / timer_period_ms) / 1024); /* F_CPU * F_IRQ / TCA_prescaler */
}
uint16_t RPM_calculation(uint16_t fancounter, uint16_t time) {
Owner

Add comments explaining the math.

Add comments explaining the math.
Author
Owner
ac5a054f8b6e04b06183bea9d740d63066d6ef22
Athamantis marked this conversation as resolved
@ -0,0 +99,4 @@
// TIMER INTERUPT
ISR (TCA0_OVF_vect) {
cli();
RPM_calculation(falling_edge_counter_1,1);
Owner

Time should use the variable and not be hardcoded.

Time should use the variable and not be hardcoded.
Author
Owner
ac5a054f8b6e04b06183bea9d740d63066d6ef22
Athamantis marked this conversation as resolved
@ -0,0 +108,4 @@
}
//-----------------------------------------------------------------------------------------------------
Owner

This setup comment should likely be removed(?)

This setup comment should likely be removed(?)
Author
Owner
ac5a054f8b6e04b06183bea9d740d63066d6ef22
Athamantis marked this conversation as resolved
@ -0,0 +1,51 @@
/*
* File: fanseeeed.h
* Author: inami
Owner

You should likely add yourself to the authors.

You should likely add yourself to the authors.
Author
Owner
ac5a054f8b6e04b06183bea9d740d63066d6ef22
Athamantis marked this conversation as resolved
@ -0,0 +27,4 @@
// INITALICE TIMER COUNTER
void init_TCA0();
// UPDATE TIMER PERIODE
Owner

The last E should be removed.

The last `E` should be removed.
Author
Owner
ac5a054f8b6e04b06183bea9d740d63066d6ef22
Athamantis marked this conversation as resolved
@ -0,0 +35,4 @@
uint16_t RPM_calculation(uint16_t fancounter, uint16_t time);
// INITIALISING FAN PORTS
void init_fanports();
Owner

fanports is a bit confusing, maybe rename to fan_gpio?

fanports is a bit confusing, maybe rename to `fan_gpio`?
Author
Owner
ac5a054f8b6e04b06183bea9d740d63066d6ef22
Athamantis marked this conversation as resolved
@ -0,0 +38,4 @@
void init_fanports();
// INIT AC0 TO COMPARE PD6 AND PD7
void init_ac0();
Owner

ac0 and ac1 should be captialized, such that they are AC0.

`ac0` and `ac1` should be captialized, such that they are `AC0`.
Author
Owner
ac5a054f8b6e04b06183bea9d740d63066d6ef22
Athamantis marked this conversation as resolved
@ -16,0 +17,4 @@
#include <stdio.h>
#include <stdlib.h>
#include "uart.h"
#include <util/delay.h>
Owner

Remove duplicate include

Remove duplicate include
Author
Owner
ac5a054f8b6e04b06183bea9d740d63066d6ef22
Athamantis marked this conversation as resolved
@ -18,0 +26,4 @@
#include <avr/io.h>
#include <avr/interrupt.h>
#include <util/delay.h>
Owner

Remove duplicate include

Remove duplicate include
Author
Owner
ac5a054f8b6e04b06183bea9d740d63066d6ef22
Athamantis marked this conversation as resolved
@ -25,3 +46,2 @@
while (1) {
uint16_t adcVal = voltage_values();
printf("The values: \n%u , %u\n",VREF_REFSEL_VDD_gc , adcVal);
//printf("loop")
Owner

Delete commented out code

Delete commented out code
Author
Owner
ac5a054f8b6e04b06183bea9d740d63066d6ef22
Athamantis marked this conversation as resolved
Inamr approved these changes 2024-04-27 16:30:05 +00:00
Athamantis added 1 commit 2024-04-27 16:45:21 +00:00
Athamantis requested review from sebgab 2024-04-27 16:48:18 +00:00
sebgab requested changes 2024-04-28 12:40:48 +00:00
sebgab left a comment
Owner

Just some small things

Just some small things
@ -0,0 +2,4 @@
#include "uart.h"
uint16_t timer_period_ms = 1;
uint16_t timer_period = 0.01;
Owner

We should not have both timer period and timer period ms. ms should be calculated from seconds, or the other way around.

We should not have both timer period and timer period ms. ms should be calculated from seconds, or the other way around.
Author
Owner
fix 772960d0467d71c2c7a88cb6743d8f6e7a7e2d69
Athamantis marked this conversation as resolved
@ -0,0 +11,4 @@
void init_TCA0() {
TCA0.SINGLE.INTCTRL = TCA_SINGLE_OVF_bm ;
TCA0.SINGLE.CTRLA = TCA_SINGLE_ENABLE_bm | TCA_SINGLE_CLKSEL_DIV1024_gc ; /* Sysclk /1024 */
//TCA0.SINGLE.PERBUF = 3906;
Owner

Remove commented out code

Remove commented out code
Author
Owner
fix 772960d0467d71c2c7a88cb6743d8f6e7a7e2d69
Athamantis marked this conversation as resolved
@ -0,0 +15,4 @@
TCA0_update_period();
}
void TCA0_update_period() {
Owner

This function should still take in the variable, not use a global variable.

This function should still take in the variable, not use a global variable.
Author
Owner
fix 772960d0467d71c2c7a88cb6743d8f6e7a7e2d69
Athamantis marked this conversation as resolved
sebgab added the due date 2024-04-29 2024-04-28 18:26:29 +00:00
Athamantis added 1 commit 2024-04-29 06:57:14 +00:00
Athamantis requested review from sebgab 2024-04-29 06:57:59 +00:00
sebgab approved these changes 2024-04-30 08:38:16 +00:00
sebgab closed this pull request 2024-04-30 09:00:13 +00:00

Pull request closed

Sign in to join this conversation.
No reviewers
No Milestone
No project
No Assignees
3 Participants
Notifications
Due Date
The due date is invalid or out of range. Please use the format 'yyyy-mm-dd'.

2024-04-29

Dependencies

No dependencies set.

Reference: Mikrokontrollersystemer-gruppe-4/mikrokontrollersystemer-prosjekt#19
No description provided.