From 49754af4cc672d8b53d52fee22d80c7bee2d8971 Mon Sep 17 00:00:00 2001 From: Oskar Weigl Date: Wed, 8 Nov 2017 23:53:15 -0800 Subject: [PATCH] add comments --- .vscode/launch.json | 8 ++------ Firmware/MotorControl/commands.cpp | 18 ++++++++++++++++++ tools/demo.py | 9 ++++++--- tools/odrive/core.py | 22 +++++++++++++++++++--- tools/odrive/protocol.py | 13 +++++++++---- tools/test_communication.py | 3 +++ 6 files changed, 57 insertions(+), 16 deletions(-) diff --git a/.vscode/launch.json b/.vscode/launch.json index 72b17470..babcd14d 100644 --- a/.vscode/launch.json +++ b/.vscode/launch.json @@ -5,16 +5,12 @@ "version": "0.2.0", "configurations": [ { - "name": "Debug Python Utility", + "name": "Python", "type": "python", "request": "launch", "stopOnEntry": true, "pythonPath": "${config:python.pythonPath}", - "program": "${workspaceRoot}/tools/test_communication.py", - "args": [ - // add specific arguments that you might wanna test - //"--serial", "/dev/ttyUSB0" - ], + "program": "${file}", "cwd": "${workspaceRoot}", "env": {}, "envFile": "${workspaceRoot}/.env", diff --git a/Firmware/MotorControl/commands.cpp b/Firmware/MotorControl/commands.cpp index ff4f919b..672eb6cc 100644 --- a/Firmware/MotorControl/commands.cpp +++ b/Firmware/MotorControl/commands.cpp @@ -198,9 +198,12 @@ constexpr size_t NUM_ENDPOINTS = sizeof(endpoints) / sizeof(endpoints[0]); // breaks our packet boundaries. For now we just neglect this. If you happen to // be limited by such a platform, you should reconsider your life choices // or as a workaround enable this: + +//Oskar: Put switches like this at top of file //#define STREAM_ON_USB + #ifdef STREAM_ON_USB class USBSender : public StreamSink { public: @@ -212,6 +215,8 @@ public: while (CDC_Transmit_FS( const_cast(buffer) /* casting this const away is safe because... well... it's not actually. Stupid STM. */, chunk) != USBD_OK) + //Oskar: we made a semaphore sem_usb_tx that guards the USB tx resource, + // that you can wait for to see if busy. Check _write in syscalls.c on devel for example use osDelay(1); buffer += chunk; length -= chunk; @@ -237,6 +242,8 @@ public: while (CDC_Transmit_FS( const_cast(buffer) /* casting this const away is safe because... well... it's not actually. Stupid STM. */, length) != USBD_OK) + //Oskar: we made a semaphore sem_usb_tx that guards the USB tx resource, + // that you can wait for to see if busy. Check _write in syscalls.c on devel for example use osDelay(1); return 0; } @@ -254,6 +261,8 @@ public: // Loop until the UART is ready // TODO: implement ring buffer to get a more continuous stream of data while (huart4.gState != HAL_UART_STATE_READY) + //Oskar: we made a semaphore sem_uart_dma that guards the UART tx resource, + // that you can wait for to see if busy. Check _write in syscalls.c on devel for example use osDelay(1); // memcpy data into uart_tx_buf memcpy(tx_buf_, buffer, length); @@ -318,12 +327,16 @@ void communication_task(void const * argument) { uint8_t c = dma_circ_buffer[last_rcv_idx]; if (++last_rcv_idx == UART_RX_BUFFER_SIZE) last_rcv_idx = 0; + //Oskar: we don't have to process 1 byte at a time, + // we can process up to MIN(last_rcv_idx, UART_RX_BUFFER_SIZE-1) UART4_stream_sink.process_bytes(&c, 1); } // When we reach here, we are out of immediate characters to fetch out of UART buffer // Now we check if there is any USB processing to do: we wait for up to 1 ms, // before going back to checking UART again. + + //Oskar: Beware of changes in devel here when merging. int USB_check_timeout = 1; int32_t status = osSemaphoreWait(sem_usb_irq, USB_check_timeout); if (status == osOK) { @@ -338,6 +351,11 @@ void communication_task(void const * argument) { vTaskDelete(osThreadGetId()); } +//Oskar: can you also do a ENABLE_LEGACY_PROTOCOL case for UART? +// If this has to be exclusive of the new protocol, that's fine: it +// lets us move on and upgrade the arduino library later. +// Please test that it still works on an arduino. + void USB_receive_packet(const uint8_t *buffer, size_t length) { //printf("[USB] got %d bytes, first is %c\r\n", length, buffer[0]); osDelay(5); #ifdef ENABLE_LEGACY_PROTOCOL diff --git a/tools/demo.py b/tools/demo.py index d8b88615..fa148eba 100755 --- a/tools/demo.py +++ b/tools/demo.py @@ -8,7 +8,10 @@ import time import math # Find a connected ODrive (this will block until you connect one) -my_drive = odrive.core.find_any(printer=print) +odrives = odrive.core.find_all(printer=print) +odrives = list(odrives) #force eval of generator to test finding functions +my_drive = odrives[0] +# my_drive = odrive.core.find_any(printer=print) # The above call returns a python object with a dynamically generated type. The # type hierarchy will correspond to the endpoint list in `MotorControl/protocol.cpp`. @@ -29,7 +32,7 @@ my_drive.motor0.set_pos_setpoint(0.0, 0.0, 0.0) # little sine wave to test t0 = time.monotonic() -while True: +while False: setpoint = 10000.0 * math.sin((time.monotonic() - t0)*2) print("goto " + str(int(setpoint))) my_drive.motor0.set_pos_setpoint(setpoint, 0.0, 0.0) @@ -39,7 +42,7 @@ while True: # Some more things you can try: # Write to a read-only property: -# my_drive.vbus_voltage = 5 # fails with `AttributeError: can't set attribute` +my_drive.vbus_voltage = 11.0 # fails with `AttributeError: can't set attribute` # Assign an incompatible value: # my_drive.motor0.pos_setpoint = "I like trains" # fails with `TypeError: expected value of type float` diff --git a/tools/odrive/core.py b/tools/odrive/core.py index 335cf638..bd528b28 100644 --- a/tools/odrive/core.py +++ b/tools/odrive/core.py @@ -68,12 +68,16 @@ def call_remote_function(channel, trigger_id, arg_properties, *args): arg_properties[i].fset(None, args[i]) channel.remote_endpoint_operation(trigger_id, None, True, 0) +#Oskar: setattr_or_raise_if_undefined def raise_if_undefined(self, name, value): """ If employed as an object's __setattr__ function, this function makes sure that an assignment to an undefined attribute doesn't create a new attribute but instead raises an exception """ + #Oskar: hasattr internally calls fget to determine if the attribute exists, + # which unnessecarily creates bus traffic. We should try to solve this. + # Step-in on the hasattr line in the debugger to see this. if hasattr(self, name): object.__setattr__(self, name, value) else: @@ -124,6 +128,8 @@ def create_property(name, json_data, channel, printer): printer("property {} has no specified ID".format(name)) return None + #Oskar: Bug: json_data calls this "access", but we look for "mode". + # The default should probably be "r" anyway, it's safer I'd say. access_mode = json_data.get("mode", "rw") return SimpleDeviceProperty(channel, id_str, property_type, struct_format, @@ -250,20 +256,30 @@ def find_usb_channels(vid_pid_pairs=odrive.util.USB_VID_PID_PAIRS, printer=nopri continue raise +def find_dev_serial_ports(search_regex): + try: + return ['/dev/' + x for x in filter(re.compile(search_regex).search, os.listdir('/dev'))] + except FileNotFoundError: + return [] + def find_serial_channels(printer=noprint): """ Scans for serial ports. Returns a generator of odrive.protocol.Channel objects. Not every returned object necessarily represents a compatible device. """ + + #Oskar: Why not just use this tool to find the available ports? + # https://pyserial.readthedocs.io/en/latest/tools.html#module-serial.tools.list_ports + # Real serial ports or USB-Serial converters - linux_real_serial_ports = ['/dev/' + x for x in filter(re.compile(r'^ttyUSB').search, os.listdir('/dev'))] + linux_real_serial_ports = find_dev_serial_ports(r'^ttyUSB') windows_real_serial_ports = [ "COM1", "COM2", "COM3", "COM4" ] # Serial devices that are exposed by the platform # for the device's USB connection - linux_usb_serial_ports = ['/dev/' + x for x in filter(re.compile(r'^ttyACM').search, os.listdir('/dev'))] - macos_usb_serial_ports = ['/dev/' + x for x in filter(re.compile(r'^tty\.usbmodem').search, os.listdir('/dev'))] + linux_usb_serial_ports = find_dev_serial_ports(r'^ttyACM') + macos_usb_serial_ports = find_dev_serial_ports(r'^tty\.usbmodem') for port in linux_real_serial_ports + windows_real_serial_ports + linux_usb_serial_ports + macos_usb_serial_ports: try: diff --git a/tools/odrive/protocol.py b/tools/odrive/protocol.py index f3460073..e66dc1f5 100644 --- a/tools/odrive/protocol.py +++ b/tools/odrive/protocol.py @@ -56,6 +56,10 @@ class ChannelBrokenException(Exception): class DeviceInitException(Exception): pass +#Oskar: I would just get rid of these "abstract classes", +# I think just looking and seeing that the classes have +# a process_packet or get_packet is enough. + class StreamSource(object): pass @@ -171,6 +175,7 @@ class Channel(PacketSink): _expected_acks = {} # Chose these parameters to be sensible for a specific transport layer + #Oskar: it's a timeout, not delay. _resend_delay = 5.0 # [s] _send_attempts = 5 @@ -203,11 +208,11 @@ class Channel(PacketSink): crc16 = calc_crc16(CRC16_INIT, packet) if (endpoint_id & 0x7fff == 0): - footer = PROTOCOL_VERSION + trailer = PROTOCOL_VERSION else: - footer = self._interface_definition_crc - #print("append footer " + footer) - packet = packet + struct.pack('