Just a few things that I found in the code for Focuserv243_DRV8825_HW203_F_OLED.ino that could be fixed or improved.
lines 1246 - 1252:
PBVal = readpbswitches(PBswitchesPin);
if ( PBVal != 0 )
{
// if a button may be pressed
delay(50); // wait 50ms in case user is holding pb down
if ( PBVal != 0 )
{
Comment:
There is no way that PBVal can change during the 50ms delay, the second "if" could be "if (true)"
line 283 (and others)
analogWrite(gledOUT, 1023 ); // indicate a pulse by lighting the green LED
line 292 (and others)
analogWrite(bledIN, 1023 ); // indicate a pulse by lighting the blue LED
Comments:
1) It's weird, but "analogWrite" only works for digital PWM pins. gledOUT and bledIN are analog pins, analog pins read analog (ADC) but can only output digital, not PWM, so this is equivalent to "digitalWrite(bledIN, 1 );"
2) analogWrite {if it did work for these pins} has acceptable values from 0-255, analogRead returns values from 0-1023.
Another comment; my buzzer wasn't working. I noticed the pin it used was never set to output. Once I did that the buzzer started working. I added a new line 1132
pinMode( Buzzer, OUTPUT );
One comment on the Windows MiniApp. The go out 50 steps button only goes out 10 steps. The other buttons all work correctly.
Aside from those things everything looks and works great. I've been making lots of little changes to tweek it for my needs, but it's great. Thank you so much for the effort on this!
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:
I have just pulled the v244 arduino code. Will post a replacement soon. Basically its not correct in checking the pb switches, soes work, but wrong logic, so am fixing the logic.
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:
I have another thought about a change to the home position switch. It would be a circuit change though, so at first I wasn't going to mention it. I was thinking that if you connected the home position switch to analog pin A6, like the pushbuttons, you could sense if the switch is attached or not. It would mean the code wouldn't need to change depending on whether the switch was used.
The reason I thought about it is I have two scopes, one is a refractor and the switch makes sense for it. The other is an SCT and it would be difficult to use a switch. I'd like to keep the code for both focusers the same. I'd already have 5v and GND at the scope for the temperature sensor, so four wires could connect the temperature and home position switch.
Hi Joe
I like the idea, I understand it requires both hardware and code change. Which is a problem as some users have already implemented the home switch.
Means changes to PCB/Stripboard layout/All firmware files/docs etc.
I understand re the refractor and reflector - which the home position switch is targeted for. Makes no sense on an SCT.
Its a fair amount of work, I'll mull over for a few days (just conscious of all the variations we have right now.... ) but it does make sense
I'd rather have v144 of the firmware also include this change, so let me test a few combinations out.
Cheers
Robert
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:
At an absolute minimum it would require all existing focuser boards to have A6 wired to 5V via 1M resistor.
Analog pins float, so the value read from the A6 pin will be indeterminate across some range, it can only goto 5V if the resistor is present, meaning if no resistor then program could think its there, hence must be fitted to all existing (and new) boards. (Even if the user doesnt fit a switch)
Only clean way to handle this (from what I can see so far) would be to keep the existing conditional compilation using #ifdef etc
Last edit: brownrb 2016-08-03
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:
Yeah, I knew it would mean a change to the circuit as well as code, so that's why I hesitated mentioning it. If any pin from A0 to A5 were available they could use an internal pull-up resistor, but A6 and A7 are pure analog, no internal pull-ups.
I don't think you should try to implement it, It wouldn't fit in with existing designs and wouldn't allow for a clean path forward. I just wanted to put the idea out there for future designs.
I would just do it in my own implementation. I've made a few other little changes to the code and circuit that suit my specific needs, but wouldn't be usefull to others. I'll just plan on having a little section of the code that I add in on my own.
Thanks for looking into it though.
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:
Just a few things that I found in the code for Focuserv243_DRV8825_HW203_F_OLED.ino that could be fixed or improved.
lines 1246 - 1252:
PBVal = readpbswitches(PBswitchesPin); if ( PBVal != 0 ) { // if a button may be pressed delay(50); // wait 50ms in case user is holding pb down if ( PBVal != 0 ) {Comment:
There is no way that PBVal can change during the 50ms delay, the second "if" could be "if (true)"
line 283 (and others)
line 292 (and others)
Comments:
1) It's weird, but "analogWrite" only works for digital PWM pins. gledOUT and bledIN are analog pins, analog pins read analog (ADC) but can only output digital, not PWM, so this is equivalent to "digitalWrite(bledIN, 1 );"
2) analogWrite {if it did work for these pins} has acceptable values from 0-255, analogRead returns values from 0-1023.
Another comment; my buzzer wasn't working. I noticed the pin it used was never set to output. Once I did that the buzzer started working. I added a new line 1132
One comment on the Windows MiniApp. The go out 50 steps button only goes out 10 steps. The other buttons all work correctly.
Aside from those things everything looks and works great. I've been making lots of little changes to tweek it for my needs, but it's great. Thank you so much for the effort on this!
Thanls for the feedback Joe, most appreciated
I will upload the corrections later today,
Cheers Robert
I have just pulled the v244 arduino code. Will post a replacement soon. Basically its not correct in checking the pb switches, soes work, but wrong logic, so am fixing the logic.
Thanks Robert. That was a very fast update!
I have another thought about a change to the home position switch. It would be a circuit change though, so at first I wasn't going to mention it. I was thinking that if you connected the home position switch to analog pin A6, like the pushbuttons, you could sense if the switch is attached or not. It would mean the code wouldn't need to change depending on whether the switch was used.
The reason I thought about it is I have two scopes, one is a refractor and the switch makes sense for it. The other is an SCT and it would be difficult to use a switch. I'd like to keep the code for both focusers the same. I'd already have 5v and GND at the scope for the temperature sensor, so four wires could connect the temperature and home position switch.
Last edit: Joe Bauer 2016-08-03
Hi Joe
I like the idea, I understand it requires both hardware and code change. Which is a problem as some users have already implemented the home switch.
Means changes to PCB/Stripboard layout/All firmware files/docs etc.
I understand re the refractor and reflector - which the home position switch is targeted for. Makes no sense on an SCT.
Its a fair amount of work, I'll mull over for a few days (just conscious of all the variations we have right now.... ) but it does make sense
I'd rather have v144 of the firmware also include this change, so let me test a few combinations out.
Cheers
Robert
Please email me Joe and I'll send you a beta file to test with your circuit.
Cheers
Robert
Hmmm, more testing
At an absolute minimum it would require all existing focuser boards to have A6 wired to 5V via 1M resistor.
Analog pins float, so the value read from the A6 pin will be indeterminate across some range, it can only goto 5V if the resistor is present, meaning if no resistor then program could think its there, hence must be fitted to all existing (and new) boards. (Even if the user doesnt fit a switch)
Only clean way to handle this (from what I can see so far) would be to keep the existing conditional compilation using #ifdef etc
Last edit: brownrb 2016-08-03
Yeah, I knew it would mean a change to the circuit as well as code, so that's why I hesitated mentioning it. If any pin from A0 to A5 were available they could use an internal pull-up resistor, but A6 and A7 are pure analog, no internal pull-ups.
I don't think you should try to implement it, It wouldn't fit in with existing designs and wouldn't allow for a clean path forward. I just wanted to put the idea out there for future designs.
I would just do it in my own implementation. I've made a few other little changes to the code and circuit that suit my specific needs, but wouldn't be usefull to others. I'll just plan on having a little section of the code that I add in on my own.
Thanks for looking into it though.