FaSTTUBe/sensor-node
36
0
Fork 0
Code Issues 7 Pull Requests 1 Packages Projects Releases Wiki Activity

WIP: interrupt based light switch detection for front SN #11

Draft
k.winkels wants to merge 1 commits from interrupt-based-ls into main
pull from: interrupt-based-ls
merge into: FaSTTUBe:main
Conversation 1 Commits 1 Files Changed 1 +54
k.winkels commented 2025-07-07 17:33:41 +02:00
Owner
Copy Link

Prototyp für #10

Leider ist diese implementierung nicht besonders flexibel, ich gehe hier davon aus das die LS in den mappings an der gleichen stelle im paket sind, und error sonst. Das geht sogar noch.
Noch uneleganter ist die lösung wo dann die Digital Reads geskipped werden und es hard-coded ist das die ersten beiden DIO_PIN_MAP einträge die light switches sein müssen.

Die EXTI line für den NVIC ist auch schwirig flexibel zu machen, 0-4 sind einzeln, 5-9 und 10-15 gruppiert, das von den mappings herzuleiten wird noch hässlicher.

Alternative zu dem versuch vom "flexiblen" ansatz ist einfach überall von dem auszugehen wie es ist (ersten beiden einträge in der DIO map und in der can signals map) und zu errorn wenn der name nicht stimmt

@o.winkels fällt dir was besseres ein?

Ungetestet, und ich mach das nochmal sauber wenn ich input für den Ansatz habe

Prototyp für #10 Leider ist diese implementierung nicht besonders flexibel, ich gehe hier davon aus das die LS in den mappings an der gleichen stelle im paket sind, und error sonst. Das geht sogar noch. Noch uneleganter ist die lösung wo dann die Digital Reads geskipped werden und es hard-coded ist das die ersten beiden DIO_PIN_MAP einträge die light switches sein müssen. Die EXTI line für den NVIC ist auch schwirig flexibel zu machen, 0-4 sind einzeln, 5-9 und 10-15 gruppiert, das von den mappings herzuleiten wird noch hässlicher. Alternative zu dem versuch vom "flexiblen" ansatz ist einfach überall von dem auszugehen wie es ist (ersten beiden einträge in der DIO map und in der can signals map) und zu errorn wenn der name nicht stimmt @o.winkels fällt dir was besseres ein? Ungetestet, und ich mach das nochmal sauber wenn ich input für den Ansatz habe
k.winkels added 1 commit 2025-07-07 17:33:42 +02:00
k.winkels requested review from o.winkels 2025-07-07 17:33:48 +02:00
k.winkels changed title from interrupt based light switch detection for front SN to WIP: interrupt based light switch detection for front SN 2025-07-07 17:34:15 +02:00
o.winkels requested changes 2025-07-07 19:12:35 +02:00
o.winkels left a comment
Owner
Copy Link

Sieht gut aus! Würde ein paar Sachen generischer implementieren aber sieht erstmal so aus als würde es funktionieren

Sieht gut aus! Würde ein paar Sachen generischer implementieren aber sieht erstmal so aus als würde es funktionieren
Software/Core/Src/main.c
@ -85,12 +88,23 @@ void send_latest_can() {
tx_counter++;
#if defined(SN_FRONT)
o.winkels commented 2025-07-07 19:02:28 +02:00
Owner
Copy Link

Vorschlag: DIO_PIN_MAP Type um ein numerisches Feld exti_idx ergänzen, -1 wenn kein interrupt. Dann

  for (int di = 0; di < NUM_DIO_PINS; di++) {
    if (DIO_PIN_MAP[di].exti_idx >= 0) {
      dio_values[di] = ls_values[DIO_PIN_MAP[di].exti_idx];
      ls_values[DIO_PIN_MAP[di].exti_idx] = 0;
    } else {
      dio_values[di] = HAL_GPIO_ReadPin(
        DIO_PIN_MAP[di].port,
        DIO_PIN_MAP[di].pin
      );
    }
  }
Vorschlag: `DIO_PIN_MAP` Type um ein numerisches Feld `exti_idx` ergänzen, -1 wenn kein interrupt. Dann ```C for (int di = 0; di < NUM_DIO_PINS; di++) { if (DIO_PIN_MAP[di].exti_idx >= 0) { dio_values[di] = ls_values[DIO_PIN_MAP[di].exti_idx]; ls_values[DIO_PIN_MAP[di].exti_idx] = 0; } else { dio_values[di] = HAL_GPIO_ReadPin( DIO_PIN_MAP[di].port, DIO_PIN_MAP[di].pin ); } } ```
o.winkels commented 2025-07-07 19:03:50 +02:00
Owner
Copy Link

Würde sowieso nicht den Begriff ls benutzen da das ja ein feature ist was unabhängig von der spezifischen Sensorik nützlich sein kann. Lieber interrupt oder irq oder exti

Würde sowieso nicht den Begriff `ls` benutzen da das ja ein feature ist was unabhängig von der spezifischen Sensorik nützlich sein kann. Lieber `interrupt` oder `irq` oder `exti`
Software/Core/Src/main.c
@ -161,0 +176,4 @@
void HAL_GPIO_EXTI_Callback(uint16_t GPIO_Pin) {
if (GPIO_Pin == DIO_PIN_MAP[CAN_SIGNAL_MAP[0].signals[0].channel].pin) {
ls_values[1] = 1;
} else if (GPIO_Pin == DIO_PIN_MAP[CAN_SIGNAL_MAP[0].signals[1].channel].pin) {
o.winkels commented 2025-07-07 18:48:41 +02:00
Owner
Copy Link

Was wenn beides gleichzeitig passiert? Würde das "else" weg lassen. Oder wären dass dann 2 gequeuete interrupts?

Was wenn beides gleichzeitig passiert? Würde das "else" weg lassen. Oder wären dass dann 2 gequeuete interrupts?
Software/Core/Src/main.c
@ -205,0 +230,4 @@
// Enable Interrupts for Light Switch Pins
#ifdef SN_FRONT
// check if pin name is "LS L"
if (strcmp(CAN_SIGNAL_MAP[0].signals[0].name, "LS L") != 0
o.winkels commented 2025-07-07 18:49:37 +02:00
Owner
Copy Link

Lieber ne Flag in dem Struct hinzufügen (oder noch besser einen neuen Signal Type definieren) als hier Strings zu vergleichen IMO

Lieber ne Flag in dem Struct hinzufügen (oder noch besser einen neuen Signal Type definieren) als hier Strings zu vergleichen IMO
o.winkels commented 2025-07-07 18:50:44 +02:00
Owner
Copy Link

Achso das ist nur ne Kontrolle. Dann okay i guess

Achso das ist nur ne Kontrolle. Dann okay i guess
Software/Core/Src/main.c
@ -205,0 +240,4 @@
GPIO_InitStruct.Pin = CAN_SIGNAL_MAP[0].signals[0].channel;
GPIO_InitStruct.Mode = GPIO_MODE_IT_RISING;
GPIO_InitStruct.Pull = GPIO_NOPULL;
HAL_GPIO_Init(DIO_PIN_MAP[CAN_SIGNAL_MAP[0].signals[0].channel].port, &GPIO_InitStruct);
o.winkels commented 2025-07-07 19:10:52 +02:00
Owner
Copy Link

Statt an all diesen Orten immer hardcoded in die Map zu indexieren würde ich eher entweder feste indizes definieren:

#define EXTI_PIN_1_PIN L9
#define EXTI_PIN_1_PIN LA

oder noch besser statt 2 separaten Init-Calls hier direkt durch ein array iterieren:

int EXTI_PINS[2] = {L9, LA};

oder noch einfacher wenn du wie oben das Attribut hinzufügst:

  for (int di = 0; di < NUM_DIO_PINS; di++) {
    if (DIO_PIN_MAP[di].exti_idx >= 0)
      HAL_GPIO_Init(...)
Statt an all diesen Orten immer hardcoded in die Map zu indexieren würde ich eher entweder feste indizes definieren: ``` #define EXTI_PIN_1_PIN L9 #define EXTI_PIN_1_PIN LA ``` oder noch besser statt 2 separaten Init-Calls hier direkt durch ein array iterieren: ``` int EXTI_PINS[2] = {L9, LA}; ``` oder noch einfacher wenn du wie oben das Attribut hinzufügst: ``` for (int di = 0; di < NUM_DIO_PINS; di++) { if (DIO_PIN_MAP[di].exti_idx >= 0) HAL_GPIO_Init(...) ```
This pull request is marked as a work in progress.
View command line instructions.

Checkout

From your project repository, check out a new branch and test the changes.
git fetch -u origin interrupt-based-ls:interrupt-based-ls
git checkout interrupt-based-ls
Sign in to join this conversation.
Reviewers
No Reviewers
o.winkels
Labels
Clear labels
bug
Something is not working
feature
New feature
idea
question
More information is needed
No Label
Milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
No Assignees
2 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: FaSTTUBe/sensor-node#11
No description provided.
Delete Branch "interrupt-based-ls"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?