nexdome / Firmware

NexDome Dome and Shutter Motor Kit Firmware
https://www.nexdome.com
Other
3 stars 6 forks source link

Feature iss #2 response encapsulation #5

Closed NameOfTheDragon closed 4 years ago

NameOfTheDragon commented 4 years ago

Ensured that all responses from the shutter are correctly encapsulated with the header ':' and terminator '#' characters

NameOfTheDragon commented 4 years ago

NexDome-Firmware-3.1.0-iss-2-response-encapsulation.27.zip Firmware for testers

knro commented 4 years ago

Just to confirm because I am waiting for access to the dome, does this also encapsulate all events include XBEE status?

knro commented 4 years ago

Ok I now have access to a dome provided by a user running the firmware in this PR. It's still not consistent:

@GAR,10
XB->Online
6120 [2400]
:right#
:GAR#
P780
P822
P889
P980
P1097
P1228
P1339
P1425
P1485
P1520
:SER,1530,0,55080,0,300#
onMotorStopped
XB->Online

IMO, all responses should start with : and end with # including the P, onMotorStopped, and XB-> status. Also, it appears that some commands returns the echo with the target included (e.g. :DRR1000#) while others omit the target from the response (e.g. :FRSemVer) when it should have been something like :FRRSemVer# or :FRSSemVer#

Is there a reason for this?

NameOfTheDragon commented 4 years ago

Thanks for the feedback. These are all valid points. Mainly, the reason for inconsistencies is that code space got very tight and I was forced to cut some corners. While it's a bit inconvenient, in a driver or application you have essentially infinite resources compared to the resource-constrained embedded firmware so I would argue that's the place where compromises should be resolved. While I agree that consistency is a worthy ideal, we are forced to accept and live with the limitations of the Arduino and the compromises that forces us to make. I'm not convinced that the cost/benefit analysis supports making further changes. We know that the firmware is usable as-is, because there's a working ASCOM driver that proves it. I will reflect on this to see if there are any improvements I can safely make and get feedback from @nexdome on whether they want me to work on this.