fossasia / pslab-firmware

Firmware for PSLab Open Hardware Platform https://pslab.io
Apache License 2.0
1.56k stars 76 forks source link

Implement get_fattime #160

Closed Hrushi20 closed 11 months ago

Hrushi20 commented 1 year ago

Pr solves Issue #159 by adding logic to write and read time from DS1307 module.

This is done by reading 32-bit unix timestamp from UART1TX register, converting it to bcd format and storing it on RTC chip via I2C protocol.

Hrushi20 commented 1 year ago

Hey, I am using a Mac M1 chip. Is there support for xc-16 compilter. I am not able to compile and check if the build succeeds or not. I tried downloading the xc-16 compiler. The cmake build fails: No xc-16 compiler found

bessman commented 1 year ago

Hey, I am using a Mac M1 chip. Is there support for xc-16 compilter. I am not able to compile and check if the build succeeds or not. I tried downloading the xc-16 compiler. The cmake build fails: No xc-16 compiler found

I am unsure how to get this working as I don't use Mac. Here is a Microchip forum thread where some people seem to have at least gotten xc8 working on M1: https://forum.microchip.com/s/topic/a5C3l000000Me63EAC/t384186

CMake uses a cmake-microchip toolchain file to detect xc16. On Mac, it searches in /Applications/microchip/. Make sure that's where you installed xc16.

Hrushi20 commented 1 year ago

Thanks!!! I got the compiler working. The build is successful. I forgot to convert bcd back to uint8_t. Need to add wrapper functions.

Hrushi20 commented 1 year ago

I didn't not add parameters to RTC_SetTime, I am reading the unix_timestamp using UART1 protocol sent by the host.

I did not understand the Wrapper functions which are to be added in commands.c file. Can you give me a brief overview of what the commands.c file does?

bessman commented 1 year ago

I didn't not add parameters to RTC_SetTime, I am reading the unix_timestamp using UART1 protocol sent by the host.

I did not understand the Wrapper functions which are to be added in commands.c file. Can you give me a brief overview of what the commands.c file does?

The main point of interest in commands.c is cmd_table. It is a 2D jump table, containing function pointers which are run in response to command instructions from the host.

When the host sends a command to the PSLab, the first two bytes are an index into this table (we refer to these bytes as "primary command" and "secondary command"). For example, if the host sends [13, 1], it means "run the function at cmd_table[13][1]".

The functions pointed to by cmd_table must have type response_t (*)(void). They take no parameters, because the functions themselves are responsible for collecting any additional data they may require from the host using UART_Read*, and send back the results using UART_Write*. However, this means these functions cannot be used internally in the firmware. For example, we want to be able to read the current time from the FatFS driver, but we cannot use RTC_GetTime because it will try to send the result to a host via UART.

Therefore, if we have a function which we want to use both internally within pslab-firmware and over serial from a host, we create a wrapper:

response_t do_something(uint8_t a, uint16_t* bp) {
    // Do something with a and write the result through bp.
}

response_t do_something_serial(void) {
    uint8_t a = UART_Read(); // Read input from host.
    uint16_t b = 0;
    response_t res = do_something(a, &b);
    UART_WriteInt(b); // Write output to host.
    return res;
}

The wrapper function do_something_serial goes in cmd_table.

Hrushi20 commented 12 months ago

Hey, Thanks for the explanation 😄. I've added the wrapper function and updated the commands.c file. What do I return to host after setting time to rtc?

Also, while importing the rtc.h header into ff_time.c, the build is failing. I imported the file using #include "../../helpers/rtc.h"

Thanks

bessman commented 12 months ago

What do I return to host after setting time to rtc?

Nothing. The return value (i.e. what is actually returned, not sent to the host) from a command function is ultimately sent to the host by the main state machine, here.

Also, while importing the rtc.h header into ff_time.c, the build is failing. I imported the file using #include "../../helpers/rtc.h"

Yes, that is how we handle includes.

Hrushi20 commented 12 months ago

What do I return to host after setting time to rtc?

Nothing. The return value (i.e. what is actually returned, not sent to the host) from a command function is ultimately sent to the host by the main state machine, here.

Also, while importing the rtc.h header into ff_time.c, the build is failing. I imported the file using #include "../../helpers/rtc.h"

Yes, that is how we handle includes.

Why is the build failing? How do I fix the issue of importing rtc.h header file?

Hrushi20 commented 12 months ago
Screenshot 2023-11-17 at 3 23 55 AM

The build fails when I import the rtc.h file into ff_time.c

bessman commented 12 months ago

Ah, sorry, I misunderstood. It fails because rtc.h is missing an include: #include "../commands.h". Not your fault; it should have already been included but I guess everything that included rtc.h before now already included commands.h themselves, so we didn't notice it was missing.

(Specifically, the response_t type, which rtc.h uses, is defined in commands.h)

Hrushi20 commented 12 months ago

Thanks. I've updated the example using rtc_gettime function.

Hrushi20 commented 11 months ago

I do not have a device to test the correctness of the code. I've updated the code as per code review 😄

bessman commented 11 months ago

I will test this tonight.

Did you get the message I sent you on Matrix? I set up a remote dev environment where you can test your changes on a PSLab.

bessman commented 11 months ago

I've done some testing on CmdSetTime and CmdGetTime, using the following Python script:

import time
from datetime import datetime
import pslab

def set_time(psl: pslab.ScienceLab, ts: int) -> int:
    psl.send_byte(13)
    psl.send_byte(1)
    psl.write(pslab.protocol.Integer.pack(ts))
    return psl.get_byte()

def get_time(psl: pslab.ScienceLab) -> int:
    psl.send_byte(13)
    psl.send_byte(3)
    ts = psl.read(4)
    psl.get_ack()
    return pslab.protocol.Integer.unpack(ts)[0]

psl = pslab.ScienceLab()
ts_in = int(time.time())
res = set_time(psl, ts)
print(res)
ts_out = get_time(psl)
print(datetime.fromtimestamp(ts_out))

Output is 2 (i.e. ArgumentError) and 2027-06-23 20:12:16, which seems a little high but since it's uninitialized might be correct.

bessman commented 11 months ago

I refined the script a bit:

import enum
import logging
import time

import pytest

import pslab

class Acknowledge(enum.Enum):
    SUCCESS = 1
    ARGUMENT_ERROR = 2
    FAILURE = 3

logger = logging.getLogger(__name__)

def set_time(psl: pslab.ScienceLab, ts: int) -> Acknowledge:
    psl.send_byte(13)  # SENSORS
    psl.send_byte(1)  # RTC_SETTIME
    logger.debug("Sent command: RTC_SETTIME")
    ser_ts = pslab.protocol.Integer.pack(ts)  # Serialize timestamp
    logger.debug(f"Serialized timestamp: {ts} -> {list(ser_ts)}")
    psl.write(ser_ts)  # Send serialized timestamp to PSLab
    logger.debug("Wrote timestamp to PSLab")
    ack = Acknowledge(psl.get_byte())
    logger.debug(f"Got ACK byte: {ack.name}")
    return ack

def get_time(psl: pslab.ScienceLab) -> int:
    psl.send_byte(13)  # SENSORS
    psl.send_byte(3)  # RTC_GETTIME
    logger.debug("Sent command: RTC_GETTIME")
    ser_ts = psl.read(4)  # Read serialized timestamp from PSLab
    logger.debug(f"Got serialized timestamp: {list(ser_ts)}")
    ack = Acknowledge(psl.get_ack())
    logger.debug(f"Got ACK byte: {ack.name}")
    ts = pslab.protocol.Integer.unpack(ser_ts)[0]  # Deserialize timestamp
    logger.debug(f"Deserialzed timestamp: {list(ser_ts)} -> {ts}")
    return ts

def test_rtc():
    psl = pslab.ScienceLab()
    logger.debug("PSLab connected")
    ts_in = int(time.time())
    logger.debug(f"Current Unix time: {ts_in}")
    # Set current time
    logger.debug("Setting time on RTC")
    ack = set_time(psl, ts_in)
    assert ack == Acknowledge.SUCCESS
    # Wait one second
    logger.debug("Waiting for one second")
    time.sleep(1)
    # Readback time
    logger.debug("Reading back time from PSLab")
    ts_out = get_time(psl)
    assert ts_out == ts_in + 1

I've added this script to the remote dev environment. To run it, just run pytest test_rtc.py after logging in. When the test passes, the RTC setter and getter functions are OK.

Hrushi20 commented 11 months ago

I am able to set the time. I am also able to fetch the time. However, there seems to be an issue with the Hour Format. Rest all matches with the time set in hardware.

Hrushi20 commented 11 months ago

I got the test working. Just tested it on the raspberry pi.

Hrushi20 commented 11 months ago

Can you review the ff_time code? I changed the logic there based on unix timestamp.

bessman commented 11 months ago

I got the test working. Just tested it on the raspberry pi.

The RTC test isn't passing for me:

________________________________________________ test_rtc ________________________________________________

    def test_rtc():
        psl = pslab.ScienceLab()
        logger.debug("PSLab connected")
        ts_in = int(time.time())
        logger.debug(f"Current Unix time: {ts_in}")
        # Set current time
        logger.debug("Setting time on RTC")
        ack = set_time(psl, ts_in)
        assert ack == Acknowledge.SUCCESS
        # Wait one second
        logger.debug("Waiting for one second")
        time.sleep(1)
        # Readback time
        logger.debug("Reading back time from PSLab")
        ts_out = get_time(psl)
>       assert ts_out == ts_in + 1
E       assert 1701649124 == (1701721123 + 1)

test_rtc.py:60: AssertionError
------------------------------------------- Captured log call --------------------------------------------
INFO     pslab.serial_handler:serial_handler.py:215 Connected to PSLab V6
 on /dev/ttyUSB0.
DEBUG    test_rtc:test_rtc.py:47 PSLab connected
DEBUG    test_rtc:test_rtc.py:49 Current Unix time: 1701721123
DEBUG    test_rtc:test_rtc.py:51 Setting time on RTC
DEBUG    test_rtc:test_rtc.py:22 Sent command: RTC_SETTIME
DEBUG    test_rtc:test_rtc.py:24 Serialized timestamp: 1701721123 -> [35, 52, 110, 101]
DEBUG    test_rtc:test_rtc.py:26 Wrote timestamp to PSLab
DEBUG    test_rtc:test_rtc.py:28 Got ACK byte: SUCCESS
DEBUG    test_rtc:test_rtc.py:55 Waiting for one second
DEBUG    test_rtc:test_rtc.py:58 Reading back time from PSLab
DEBUG    test_rtc:test_rtc.py:35 Sent command: RTC_GETTIME
DEBUG    test_rtc:test_rtc.py:37 Got serialized timestamp: [228, 26, 109, 101]
DEBUG    test_rtc:test_rtc.py:39 Got ACK byte: SUCCESS
DEBUG    test_rtc:test_rtc.py:41 Deserialzed timestamp: [228, 26, 109, 101] -> 1701649124
======================================== short test summary info =========================================
FAILED test_rtc.py::test_rtc - assert 1701649124 == (1701721123 + 1)
=========================================== 1 failed in 2.81s ============================================
bessman commented 11 months ago

Can you review the ff_time code? I changed the logic there based on unix timestamp.

I've added another test script, test_fattime.py, which you can run in the same way as the other test script. It sets the RTC time, touches a file on the SD-card, and then reads the file's modification timestamp.

It relies on some recently added SD-card functionality, so you will need to rebase your branch on main to get it to work.

You will also need to run cmake with the -DLEGACY_HARDWARE=1 flag. This is because the PSLab which is connected to the RPi is a prototype of the upcoming v6 hardware. (This wasn't necessary before because the RTC is the same in both hardware versions. The SD-card, on the other hand, is implemented differently between hardware versions)

Right now I'm getting this test result (after locally merging this PR into main):

______________________________________________ test_fattime ______________________________________________

    def test_fattime():
        psl = pslab.ScienceLab()
        sdcard = SDCard(psl)
        now = int(time.time())
        set_time(psl, now)
        logger.debug(f"Current Unix time: {now}")
        logger.debug("Touching file")
        sdcard.write_file("TIMETEST.TXT", b"", Mode.FA_CREATE_ALWAYS)
        time.sleep(0.1)
        logger.debug("Reading file metadata")
        _, fdate, ftime, _ = sdcard.get_info("TIMETEST.TXT")
        modification_ts = datetime(*fattime2datetime(fdate, ftime))
        logger.debug(f"Modification date: {modification_ts}")
>       assert int(modification_ts.timestamp()) == now
E       assert 3573844872 == 1701722473
E        +  where 3573844872 = int(3573844872.0)
E        +    where 3573844872.0 = <built-in method timestamp of datetime.datetime object at 0x7f8a26eb20>()
E        +      where <built-in method timestamp of datetime.datetime object at 0x7f8a26eb20> = datetime.datetime(2083, 4, 2, 0, 41, 12).timestamp

test_fattime.py:192: AssertionError
------------------------------------------ Captured stdout call ------------------------------------------
open: FR_OK
------------------------------------------- Captured log call --------------------------------------------
INFO     pslab.serial_handler:serial_handler.py:215 Connected to PSLab V6
 on /dev/ttyUSB0.
DEBUG    test_fattime:test_fattime.py:146 Sent command: RTC_SETTIME
DEBUG    test_fattime:test_fattime.py:148 Serialized timestamp: 1701722473 -> [105, 57, 110, 101]
DEBUG    test_fattime:test_fattime.py:150 Wrote timestamp to PSLab
DEBUG    test_fattime:test_fattime.py:152 Got ACK byte: SUCCESS
DEBUG    test_fattime:test_fattime.py:184 Current Unix time: 1701722473
DEBUG    test_fattime:test_fattime.py:185 Touching file
DEBUG    test_fattime:test_fattime.py:188 Reading file metadata
DEBUG    test_fattime:test_fattime.py:191 Modification date: 2083-04-02 00:41:12
======================================== short test summary info =========================================
FAILED test_fattime.py::test_fattime - assert 3573844872 == 1701722473
=========================================== 1 failed in 2.07s ============================================
Hrushi20 commented 11 months ago

I got the test working. Just tested it on the raspberry pi.

The RTC test isn't passing for me:

________________________________________________ test_rtc ________________________________________________

    def test_rtc():
        psl = pslab.ScienceLab()
        logger.debug("PSLab connected")
        ts_in = int(time.time())
        logger.debug(f"Current Unix time: {ts_in}")
        # Set current time
        logger.debug("Setting time on RTC")
        ack = set_time(psl, ts_in)
        assert ack == Acknowledge.SUCCESS
        # Wait one second
        logger.debug("Waiting for one second")
        time.sleep(1)
        # Readback time
        logger.debug("Reading back time from PSLab")
        ts_out = get_time(psl)
>       assert ts_out == ts_in + 1
E       assert 1701649124 == (1701721123 + 1)

test_rtc.py:60: AssertionError
------------------------------------------- Captured log call --------------------------------------------
INFO     pslab.serial_handler:serial_handler.py:215 Connected to PSLab V6
 on /dev/ttyUSB0.
DEBUG    test_rtc:test_rtc.py:47 PSLab connected
DEBUG    test_rtc:test_rtc.py:49 Current Unix time: 1701721123
DEBUG    test_rtc:test_rtc.py:51 Setting time on RTC
DEBUG    test_rtc:test_rtc.py:22 Sent command: RTC_SETTIME
DEBUG    test_rtc:test_rtc.py:24 Serialized timestamp: 1701721123 -> [35, 52, 110, 101]
DEBUG    test_rtc:test_rtc.py:26 Wrote timestamp to PSLab
DEBUG    test_rtc:test_rtc.py:28 Got ACK byte: SUCCESS
DEBUG    test_rtc:test_rtc.py:55 Waiting for one second
DEBUG    test_rtc:test_rtc.py:58 Reading back time from PSLab
DEBUG    test_rtc:test_rtc.py:35 Sent command: RTC_GETTIME
DEBUG    test_rtc:test_rtc.py:37 Got serialized timestamp: [228, 26, 109, 101]
DEBUG    test_rtc:test_rtc.py:39 Got ACK byte: SUCCESS
DEBUG    test_rtc:test_rtc.py:41 Deserialzed timestamp: [228, 26, 109, 101] -> 1701649124
======================================== short test summary info =========================================
FAILED test_rtc.py::test_rtc - assert 1701649124 == (1701721123 + 1)
=========================================== 1 failed in 2.81s ============================================

Hey, can you flash and try again. Are you using the latest commit? The test is passing for me.

Hrushi20 commented 11 months ago
Screenshot 2023-12-05 at 9 47 23 AM
Hrushi20 commented 11 months ago

The test_fattime fails. There is an issue with the hours format. I'll debug it during ny free time.

bessman commented 11 months ago

Hey, can you flash and try again. Are you using the latest commit? The test is passing for me.

Now it's working :+1:

The test_fattime fails. There is an issue with the hours format. I'll debug it during ny free time.

It was a problem with the test; It didn't take the timezone into account. I've fixed it and both tests are passing. Great work!

I'll give this a final look tonight, but it seems ready for merge.

Hrushi20 commented 11 months ago

I'm looking forward to the merge 😁