Lawebb - timing updates by dfrencham · Pull Request #3 · dfrencham/rad-gate

dfrencham · 2018-03-05T21:10:41Z

Code provided by lawebb as example of timing implementation

dfrencham · 2018-03-05T21:12:46Z

LightTree.cpp

-    digitalWrite(PIN_LIGHT_TREE_RELAY_2, HIGH);
-    digitalWrite(PIN_LIGHT_TREE_RELAY_3, HIGH);
-    digitalWrite(PIN_LIGHT_TREE_RELAY_4, HIGH);
+    digitalWrite(PIN_LIGHT_TREE_RELAY_1, LOW);


We need a new configuration option to reverse relay states.

dfrencham · 2018-03-05T21:13:34Z

Makefile

 # Arduino Make file. Refer to https://github.com/sudar/Arduino-Makefile

-MONITOR_PORT  = /dev/cu.wchusbserial1420
+MONITOR_PORT  = /dev/ttyACM0


You must be using a better quality Arduino than me :)

dfrencham · 2018-03-05T21:16:39Z

Sequence.cpp

+  4  240     Set LED3  Play Tone 632 
+  5  300               Stop Tone 
+  6  360     Set LED4  Play Tone 632   Gate Activate
+  7  660                               Gate Deactivate


I'm not sure why we need an explicit "Deactivate" state. After the gate drops it is deactivated (magnet off, no sequence running). I can understand an "armed" state, as that implies the magnet is powered.

The solenoid just requires a pulse. 300 ms seems long enough. Leaving it on sucks a lot of power and will surely burn out the coil. I tried making a pulse by dumping charge from 50000uF worth of charge from some capacitors - not enough. So software is better, and the need to deactivate. The The Gate::arm() function was empty so used it... should have renamed it

ok, cheers makes sense. Should we have a variable name gateControlType that we can set to "solenoid" (with default being "magnet")?

That way we can change the behaviour based on that setting.

dfrencham · 2018-03-05T21:20:06Z

Sequence.cpp

+  gate->set_abortable(false); 
+
+  // Use interrupt to catch END request from SENSOR
+  attachInterrupt(digitalPinToInterrupt(PIN_SENSOR), Interrupt2, LOW);


You don't seem to be detaching the interrupt anywhere - so this interrupt could be attached multiple times. The results might be unexpected after the first run.

OK - haven't noticed any problem as yet

dfrencham · 2018-03-05T21:21:19Z

Sequence.h

    void begin_sequence();
    void abort_seq();
    void set_ready();
+    void setLed(int led_val);


not sure why setLed is here - you can do it via the LightTree reference.

I wrote a test sequence.cpp , and did cut and past when was working. Then slavishly copied the setLed, gateAct, and playTone functions too. Agreed can get rid of them

dfrencham · 2018-03-05T21:21:56Z

Sequence.h

    void abort_seq();
    void set_ready();
+    void setLed(int led_val);
+    void gateAct(int gate_val);


use Gate class reference

dfrencham · 2018-03-05T21:22:47Z

Sequence.h

    void set_ready();
+    void setLed(int led_val);
+    void gateAct(int gate_val);
+    void playTone(int play_val);


use AudioFX class reference

dfrencham · 2018-03-05T21:23:16Z

constants.h

 #define DELAY_RAND_MAX 2700

+// some inferior gate controllers don't BEEP at the correct pitch
+// frequence. Correct tones (in Hz) are below


Thanks, I had this in the wrong place.

dfrencham · 2018-03-05T21:25:23Z

constants.h

 #define PIN_SFX_ACT 1 // sound board active

-#define PIN_LIGHT_TREE_RELAY_ENABLE 3
+#define PIN_LIGHT_TREE_RELAY_ENABLE 4


I know we have at least one bloke running a gate that expects to enable the light tree with pin 3. We might need to find another solution (we could use Pin 19).

Pins 2 and 3 are the only ones that can be used for an interrupt input by my reading. 2 is used for ABORT so had to use 3 for the interrupt for the light beam

argh! you're right. I might have to make light tree mode a software configuration item rather than a jumper.

We should be able to listen for the beam while the sequence is running. As the sequence loop is still active, we can check at the start of each loop if the beam pin has gone low. As the ATMEGA is already busy running that loop, we won't run into any issues by doing the check there.

Do you mean to not use an interrupt? here are two examples of serial monitor using test button - first button is before sequence is finished - second is after finished.

[17430] No longer abortable
[17430] Set LED 1
[17550] Set LED 2
[17670] Set LED 3
[17790] Set LED 4
[17790] Gate Activate
[18091] Gate Deactivate
[20013] Reaction Time = 2223
[20013] Press GO to restart...
[20040] Setting sequence to STOP

[89785] No longer abortable
[89785] Set LED 1
[89905] Set LED 2
[90025] Set LED 3
[90145] Set LED 4
[90145] Gate Activate
[90445] Gate Deactivate
[92395] Setting sequence to STOP
[92395] Sequence complete - Waiting for reaction time...
[94418] Reaction Time = 4273
[94418] Press GO to restart...

Yeah - should be do-able without an interrupt. We can keep the sequence loop running until we have a time. Pseudo code:

while (sequenceRunning || (timingEnabled && waitingForTime)) { if (timingEnabled && gateHasDropped) { if (digitalRead(PIN_TIMING_TRIGGER) == LOW) { // time found, save in var waitingForTime = 0; } } // rest of sequence code }

dfrencham · 2018-03-05T21:27:05Z

rad-gate.ino


  serial_print("Gate controller initialised");
-  serial_print(VERSION);
+  serial_print("Waiting for GO Button Press");


We should keep the version string print, but also add your line. Otherwise it isn't clear we are ready for a button press. Good idea.

dfrencham · 2018-03-05T21:37:58Z

Sequence.cpp

+  attachInterrupt(digitalPinToInterrupt(PIN_SENSOR), Interrupt2, LOW);
+
+  offset = millis();
+


I was confused with how this was structured until I realised you are running a series of tasks as an inner loop for each step.

dfrencham · 2018-03-05T23:02:19Z

Sequence.cpp

+
+  offset = millis();
+
+  while((step < count) && (!gate->is_aborted())) {


@lawabb what was the reason for the refactor with the loop? was the original code difficult / unwieldily to update? (if the original code was rubbish, please say so!)

The original code was fine. It took me sometime to figure out how it worked, but I successfully modified it to work with my addition of 'gate deactivate' with the exception that I couldn't set wait time to zero so had to use 1ms. The trouble was that it was becoming unweildy and ugly... I don't know how to merge with the original code. As an amateur python programmer what I did was write the sequence in python the transferred it virtually line by line with help from google. It may be of some use...
test_new_seq.zip

denisborgesalves

Alguém tem o código-fonte mais recente da UCI para me fornecer?

Meu e-mail é:

denisborgesalves@gmail.com

dfrencham added 3 commits March 5, 2018 16:21

Makefile update

06ef7b1

Update .gitignore file

f81bee8

lawabb - timing updates

79580f0

dfrencham commented Mar 5, 2018

View reviewed changes

dfrencham mentioned this pull request Mar 5, 2018

Add timing method #2

Open

dfrencham commented Mar 5, 2018

View reviewed changes

denisborgesalves reviewed Mar 19, 2018

View reviewed changes

		attachInterrupt(digitalPinToInterrupt(PIN_SENSOR), Interrupt2, LOW);

		offset = millis();


		offset = millis();

		while((step < count) && (!gate->is_aborted())) {

Conversation

dfrencham commented Mar 5, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

denisborgesalves left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants