Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement get_fattime #160

Merged
merged 19 commits into from
Dec 5, 2023
Merged

Implement get_fattime #160

merged 19 commits into from
Dec 5, 2023

Conversation

Hrushi20
Copy link
Contributor

@Hrushi20 Hrushi20 commented Nov 14, 2023

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
Copy link
Contributor Author

Hrushi20 commented Nov 14, 2023

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
Copy link
Collaborator

bessman commented Nov 14, 2023

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.

Signed-off-by: Hrushi20 <[email protected]>
@Hrushi20
Copy link
Contributor Author

Hrushi20 commented Nov 14, 2023

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
Copy link
Contributor Author

Hrushi20 commented Nov 14, 2023

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
Copy link
Collaborator

bessman commented Nov 14, 2023

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
Copy link
Contributor Author

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
Copy link
Collaborator

bessman commented Nov 16, 2023

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.

src/helpers/rtc.c Outdated Show resolved Hide resolved
@Hrushi20
Copy link
Contributor Author

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
Copy link
Contributor Author

Screenshot 2023-11-17 at 3 23 55 AM

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

@bessman
Copy link
Collaborator

bessman commented Nov 16, 2023

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
Copy link
Contributor Author

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

Copy link
Collaborator

@bessman bessman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi, I haven't had time to try this out yet, but here are some general comments.

src/commands.c Outdated Show resolved Hide resolved
src/helpers/rtc.c Outdated Show resolved Hide resolved
src/helpers/rtc.c Outdated Show resolved Hide resolved
src/helpers/rtc.h Outdated Show resolved Hide resolved
src/sdcard/fatfs/ff_time.c Show resolved Hide resolved
Copy link
Collaborator

@bessman bessman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Almost there :)

src/helpers/rtc.c Outdated Show resolved Hide resolved
src/helpers/rtc.c Outdated Show resolved Hide resolved
src/helpers/rtc.c Outdated Show resolved Hide resolved
src/sdcard/fatfs/ff_time.c Outdated Show resolved Hide resolved
@Hrushi20
Copy link
Contributor Author

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

Signed-off-by: Hrushi20 <[email protected]>
@bessman
Copy link
Collaborator

bessman commented Nov 30, 2023

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
Copy link
Collaborator

bessman commented Nov 30, 2023

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
Copy link
Collaborator

bessman commented Dec 1, 2023

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.

src/helpers/rtc.c Outdated Show resolved Hide resolved
@Hrushi20
Copy link
Contributor Author

Hrushi20 commented Dec 1, 2023

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
Copy link
Contributor Author

Hrushi20 commented Dec 1, 2023

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

@Hrushi20
Copy link
Contributor Author

Hrushi20 commented Dec 1, 2023

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

@bessman
Copy link
Collaborator

bessman commented Dec 4, 2023

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
Copy link
Collaborator

bessman commented Dec 4, 2023

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 ============================================

src/sdcard/fatfs/ff_time.c Outdated Show resolved Hide resolved
src/sdcard/fatfs/ff_time.c Outdated Show resolved Hide resolved
src/sdcard/fatfs/ff_time.c Outdated Show resolved Hide resolved
src/helpers/rtc.c Outdated Show resolved Hide resolved
src/helpers/rtc.c Outdated Show resolved Hide resolved
@Hrushi20
Copy link
Contributor Author

Hrushi20 commented Dec 5, 2023

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
Copy link
Contributor Author

Hrushi20 commented Dec 5, 2023

Screenshot 2023-12-05 at 9 47 23 AM

Signed-off-by: Hrushi20 <[email protected]>
@Hrushi20
Copy link
Contributor Author

Hrushi20 commented Dec 5, 2023

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

@bessman
Copy link
Collaborator

bessman commented Dec 5, 2023

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

Now it's working 👍

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
Copy link
Contributor Author

Hrushi20 commented Dec 5, 2023

I'm looking forward to the merge 😁

@bessman bessman changed the title Added features to #159 Implement get_fattime Dec 5, 2023
@bessman bessman merged commit 80f021d into fossasia:main Dec 5, 2023
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants