Skip to content

Added changes for ls-l time support #3406

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.

Sign up for GitHub

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

Open
Biancaa-R wants to merge 1 commit into
base: master
Choose a base branch
from
+124 −2
Open

Added changes for ls-l time support #3406

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
126 changes: 124 additions & 2 deletions nshlib/nsh_fscmds.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,9 +44,12 @@
#include <libgen.h>
#include <errno.h>
#include <debug.h>

#include "nsh.h"

#ifdef CONFIG_ARCH_SIM
Copy link
Contributor

@xiaoxiang781216 xiaoxiang781216 Feb 24, 2026

Choose a reason for hiding this comment

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

why check CONFIG_ARCH_SIM

Copy link
Author

@Biancaa-R Biancaa-R Feb 24, 2026

Choose a reason for hiding this comment

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

As I had mentioned , the logic I had built before ,including time.h for all cases was too much flash memory (xip code ,data ) for a particular stm board that had 128KB flash . So I replaced the logic for manual calculation of datetime from epoch instead of using localtime_r(&t, &tm) .

That logic will work in microcontrollers that have RTc and have kept TIMEKEEPING configured.
In simulation , it doesnt work because of : #3410 .

So I tried to fix it by gettime , settime functions (Only in case SIM is used) and since there is enough memory to handle it .

But I have to remove it as it was pointed out by @linguini1 that its not the work of ls hander to display accurate date but , the date given by the system.

or we have to manually modify once before creating files in SIM.

nsh> date
Thu, Jan 22 18:08:41 2004 
nsh> date -s "Feb 23 22:35:00 2026"

Copy link
Contributor

@xiaoxiang781216 xiaoxiang781216 Feb 26, 2026

Choose a reason for hiding this comment

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

it's simple to call ctime_r or strftime and print out the result, I don't understand why do you need any other code.

# include <time.h>
#endif

#if !defined(CONFIG_DISABLE_MOUNTPOINT)
# include <sys/mount.h>
# include <sys/boardctl.h>
Expand Down Expand Up @@ -364,7 +367,126 @@ static int ls_handler(FAR struct nsh_vtbl_s *vtbl, FAR const char *dirpath,

memset(&buf, 0, sizeof(struct stat));

/* stat the file */
/* If entryp is provided, listing a directory and need to
* construct the full path to stat the file. Otherwise, dirpath
* is the target itself. (no separate file name as entryp)
*/

if (entryp != NULL)
Copy link
Contributor

@xiaoxiang781216 xiaoxiang781216 Feb 24, 2026

Choose a reason for hiding this comment

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

why dup with line 494

Copy link
Author

@Biancaa-R Biancaa-R Feb 24, 2026

Choose a reason for hiding this comment

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

Sure Ill move it to the top so it happens once . This one gets executed in case of ls -l specifically.

{
FAR char *fullpath = nsh_getdirpath(vtbl, dirpath, entryp->d_name);
ret = stat(fullpath, &buf);
free(fullpath);
}
else
{
ret = stat(dirpath, &buf);
}

#ifdef CONFIG_CLOCK_TIMEKEEPING
/* manual epoch time to date calculation to reduce extra memory
* by using includes For boards with minimal flash.
*/

# ifdef CONFIG_ARCH_SIM
struct timespec ts;

/* This pulls the ACTUAL current time from your Linux Host */

/* Sometime defaults to 2008 */

if (clock_gettime(CLOCK_REALTIME, &ts) == 0)
{
clock_settime(CLOCK_REALTIME, &ts);
Copy link
Contributor

@xiaoxiang781216 xiaoxiang781216 Feb 24, 2026

Choose a reason for hiding this comment

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

why do you change host time?

}
# endif
Comment on lines +386 to +402
Copy link
Contributor

@acassis acassis Feb 23, 2026

Choose a reason for hiding this comment

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

I think this part is the same for SIM and for native boards. Could you please confirm it in some ESP32 or STM32 board? If that is true we don't need this #ifdef CONFIG_ARCH_SIM

Copy link
Author

@Biancaa-R Biancaa-R Feb 23, 2026

Choose a reason for hiding this comment

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

Sure sir Ill check it with a MC.

Copy link
Contributor

@acassis acassis Feb 23, 2026

Choose a reason for hiding this comment

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

@Biancaa-R sorry, I just saw now that this is a hack to fix the 2008 issue. In fact it happened in my test (that don't have your PR applied), see below


/* for referencing /data : if not mounted the reference will fail. */

# if defined(CONFIG_ARCH_SIM) && defined(CONFIG_FS_HOSTFS)
static bool g_time_synced = false;

if (!g_time_synced)
{
struct stat hstat;

/* Data section always maintains the current time value. */

if (stat("/data", &hstat) == 0)
{
struct timespec ts_sync;

ts_sync.tv_sec = hstat.st_mtime;
ts_sync.tv_nsec = 0;

/* This is the magic line that sets the system clock */

if (clock_settime(CLOCK_REALTIME, &ts_sync) == 0)
Copy link
Contributor

@xiaoxiang781216 xiaoxiang781216 Feb 24, 2026

Choose a reason for hiding this comment

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

ditto

Copy link
Author

@Biancaa-R Biancaa-R Feb 24, 2026 •
edited
Loading

Choose a reason for hiding this comment

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

No this one tries to set the time of /data section as the time pulled from the linux host in SIM defaults to 2004/ 2008 ,In case a hostfs is present /data always takes the accurate time , so I am keeping it as a reference here in this place. Look at : https://github.com/apache/nuttx-apps/pull/3406/changes/BASE..3ae3781f8844d1d3f6fa3b6a96b3515a48129f72#r2842897622

Copy link
Contributor

@xiaoxiang781216 xiaoxiang781216 Feb 26, 2026

Choose a reason for hiding this comment

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

first, you should drop all sim specific code from nsh, it's wrong to add any arch specific code in the application. second, I don't understand why do you change realtime in ls command it's definitely wrong. before you make this hack, explain what's problem you encounter first.

Copy link
Author

@Biancaa-R Biancaa-R Feb 26, 2026

Choose a reason for hiding this comment

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

Yes sir , as @linguini1 sir pointed we dont have to modify the displayed time in ls , even if its wrong. (only have ls functionality.) So I'll REMOVE SIM specific code . I 'll make the changes and push it.

For the issue I encountered please look at : #3406 (comment)


 nsh> echo "hey" > hello.txt

nsh> ls -l

/:

 ----/--/-- --:-- dr-xr-xr-x           0 bin/

 2026-02-23 16:58:38 drwxrwxr-x        4096 data/

 2008-06-01 00:00:00 drwxrwxrwx           0 dev/

 ----/--/-- --:-- dr-xr-xr-x           0 etc/

 2004-01-22 18:08:50 -rw-rw-rw-           4 hello.txt

 2004-01-22 18:08:46 -rw-rw-rw-           3 hi.txt

 ----/--/-- --:-- dr--r--r--           0 proc/

 ----/--/-- --:-- drw-rw-rw-           0 tmp/

Using clk_gettime for puling current time from Linux Host) sometimes gave me faulty results from the system clock ,where the system clock often defaults to the Unix epoch or a fixed build date (like 2008/2004).

  • This is because in HostFS the data is dynamic and has access to system's clk -> So it will get correct time.
  • All other sections and the files we create , assumes this base time (which is not accurate.)
  • Unlike other microcontrollers we dont have a RTC here.

SO I'll remove the SIM specific code ,and push it again ,please give me some time ,Ill fix it and get back ,

{
g_time_synced = true;
}
}
}
# endif
Comment on lines +406 to +430
Copy link
Contributor

@acassis acassis Feb 23, 2026

Choose a reason for hiding this comment

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

I tested this hack on sim:nsh (that has CONFIG_FS_HOSTFS enabled by default) and it worked.

In fact my test was very simple, I modified the hello example to:

#include <time.h>
#include <sys/stat.h>

....

int main(int argc, FAR char *argv[])
{
  struct stat hstat;
	
  /* Data section always maintains the current time value. */
	
  if (stat("/data", &hstat) == 0)
    {
       struct timespec ts_sync;
	
       ts_sync.tv_sec  = hstat.st_mtime;
       ts_sync.tv_nsec = 0;

       /* This is the magic line that sets the system clock */

      if (clock_settime(CLOCK_REALTIME, &ts_sync) == 0)
        {
		  return 0;
        }
    }

	return -1;
}

Then I ran ./nuttx and tested this way:

NuttShell (NSH) NuttX-12.12.0
nsh> date
Sun, Jun 01 00:00:01 2008
nsh> hello
nsh> echo $?
0
nsh> date
Mon, Feb 23 20:15:11 2026
nsh>

So, this hack should be in the #else of CONFIG_CLOCK_TIMEKEEPING

Copy link
Contributor

@acassis acassis Feb 23, 2026

Choose a reason for hiding this comment

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

@Biancaa-R "/* This is the magic line that sets the system clock */" Sounds very AI generated, please use: "Now, set the system clock with the time value got /data"

Copy link
Contributor

@acassis acassis Feb 23, 2026

Choose a reason for hiding this comment

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

@Biancaa-R just remove all hacks code related SIM, just keep the code to display the date on "ls -l".
For native boards, I think when RTC and CONFIG_CLOCK_TIMEKEEPING is not enabled the date will be wrong, so in this case, instead of showing an old invalid date, just display "--/--/----"


if (ret == 0 && buf.st_mtime > 0 && buf.st_mtime < 0x7fffffff)
Copy link
Contributor

@xiaoxiang781216 xiaoxiang781216 Feb 24, 2026

Choose a reason for hiding this comment

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

call strftime instead

Copy link
Author

@Biancaa-R Biancaa-R Feb 24, 2026

Choose a reason for hiding this comment

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

I dont know , but strftime() is a heavy function that pulls in a lot of the standard C library (libc). Many NuttX boards are configured with CONFIG_LIBC_LOCALTIME disabled to save space. The main reason I added a manual calculation for epoch to datetime is to save space . please look at : #3406 (comment)

Copy link
Contributor

@xiaoxiang781216 xiaoxiang781216 Feb 26, 2026

Choose a reason for hiding this comment

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

but strftime doesn't require you enble CONFIG_LIBC_LOCALTIME,

Copy link
Author

@Biancaa-R Biancaa-R Feb 26, 2026

Choose a reason for hiding this comment

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

Oh okay , but it will still need <time.h> , Ill calculate the final size @xiaoxiang781216 , if It fits in the mem of 128KB flash of stm ,I'll add that instead . Thank you.

{
uint32_t seconds = (uint32_t)buf.st_mtime;
uint32_t minutes = seconds / 60;
uint32_t hours = minutes / 60;
uint32_t days = hours / 24;
uint32_t y = 1970;
uint32_t m = 0;

while (1)
{
uint32_t diy = (y % 4 == 0 && (y % 100 != 0 ||
y % 400 == 0)) ? 366 : 365;

if (days < diy)
{
break;
}

days -= diy;
y++;
}

static const uint8_t dim[] =
{
31, 28, 31, 30, 31, 30, 31, 31, 30, 31, 30, 31
};

for (m = 0; m < 12; m++)
{
uint32_t d_m = dim[m];

if (m == 1 && (y % 4 == 0 && (y % 100 != 0 || y % 400 == 0)))
{
d_m = 29;
}

if (days < d_m)
{
break;
}

days -= d_m;
}

nsh_output(vtbl, " %04u-%02u-%02u %02u:%02u:%02u",
Copy link
Contributor

@xiaoxiang781216 xiaoxiang781216 Feb 24, 2026

Choose a reason for hiding this comment

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

should move just before file name like bash:

drwxr-xr-x 2 xiaoxiang xiaoxiang 4096 Feb 15 11:02 cmake_with_components
drwxr-xr-x 3 xiaoxiang xiaoxiang 4096 Feb 15 11:02 legacy_cmake_relpath
drwxr-xr-x 2 xiaoxiang xiaoxiang 4096 Feb 15 11:02 legacy_cmake_simple
```

(unsigned int)y, (unsigned int)m + 1,
(unsigned int)days + 1, (unsigned int)(hours % 24),
(unsigned int)(minutes % 60),
(unsigned int)(seconds % 60));
}
else
{
/* Incase valid time entry was not maintained in the stat struct. */

nsh_output(vtbl, " ----/--/-- --:--");
}
#endif

if (entryp != NULL)
{
Expand Down
Loading